curator-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dragonsi...@apache.org
Subject [3/6] git commit: Fix formatting; remove MyTreeCache; error listener
Date Mon, 06 Oct 2014 23:52:26 GMT
Fix formatting; remove MyTreeCache; error listener


Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/180a1598
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/180a1598
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/180a1598

Branch: refs/heads/master
Commit: 180a1598f7b1550c6cab65bea81053e2d0a0ae60
Parents: 825a962
Author: Scott Blum <scottb@squareup.com>
Authored: Tue Aug 26 19:51:29 2014 -0400
Committer: Scott Blum <scottb@squareup.com>
Committed: Tue Aug 26 19:51:29 2014 -0400

----------------------------------------------------------------------
 .../framework/recipes/cache/TreeCache.java      | 77 ++++++++++++++------
 .../recipes/cache/BaseTestTreeCache.java        | 77 +++++++++++---------
 .../framework/recipes/cache/TestTreeCache.java  | 36 +++++----
 .../recipes/cache/TestTreeCacheRandomTree.java  |  4 +-
 4 files changed, 122 insertions(+), 72 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/180a1598/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 030ea2c..3a7a1de 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
@@ -26,6 +26,8 @@ import com.google.common.collect.Maps;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.api.BackgroundCallback;
 import org.apache.curator.framework.api.CuratorEvent;
+import org.apache.curator.framework.api.UnhandledErrorListener;
+import org.apache.curator.framework.listen.Listenable;
 import org.apache.curator.framework.listen.ListenerContainer;
 import org.apache.curator.framework.state.ConnectionState;
 import org.apache.curator.framework.state.ConnectionStateListener;
@@ -67,14 +69,16 @@ public class TreeCache implements Closeable
 {
     private static final Logger LOG = LoggerFactory.getLogger(TreeCache.class);
 
-    public static final class Builder {
+    public static final class Builder
+    {
         private final CuratorFramework client;
         private final String path;
         private boolean cacheData = true;
         private boolean dataIsCompressed = false;
         private CloseableExecutorService executorService = null;
 
-        private Builder(CuratorFramework client, String path) {
+        private Builder(CuratorFramework client, String path)
+        {
             this.client = checkNotNull(client);
             this.path = validatePath(path);
         }
@@ -133,9 +137,12 @@ public class TreeCache implements Closeable
          */
         public Builder setExecutor(ExecutorService executorService)
         {
-            if (executorService instanceof CloseableExecutorService) {
-                return setExecutor((CloseableExecutorService) executorService);
-            } else {
+            if ( executorService instanceof CloseableExecutorService )
+            {
+                return setExecutor((CloseableExecutorService)executorService);
+            }
+            else
+            {
                 return setExecutor(new CloseableExecutorService(executorService));
             }
         }
@@ -152,18 +159,19 @@ public class TreeCache implements Closeable
 
     /**
      * Create a TreeCache builder for the given client and path to configure advanced options.
-     *
+     * <p/>
      * If the client is namespaced, all operations on the resulting TreeCache will be in
terms of
      * the namespace, including all published events.  The given path is the root at which
the
      * TreeCache will watch and explore.  If no node exists at the given path, the TreeCache
will
      * be initially empty.
      *
      * @param client the client to use; may be namespaced
-     * @param path the path to the root node to watch/explore; this path need not actually
exist on
-     *             the server
+     * @param path   the path to the root node to watch/explore; this path need not actually
exist on
+     *               the server
      * @return a new builder
      */
-    public static Builder newBuilder(CuratorFramework client, String path) {
+    public static Builder newBuilder(CuratorFramework client, String path)
+    {
         return new Builder(client, path);
     }
 
@@ -333,7 +341,8 @@ public class TreeCache implements Closeable
                 if ( event.getResultCode() == KeeperException.Code.OK.intValue() )
                 {
                     Stat oldStat = stat.get();
-                    if (oldStat != null && oldStat.getMzxid() == newStat.getMzxid())
{
+                    if ( oldStat != null && oldStat.getMzxid() == newStat.getMzxid()
)
+                    {
                         // Only update stat if mzxid is different, otherwise we might obscure
                         // GET_DATA event updates.
                         stat.set(newStat);
@@ -447,6 +456,7 @@ public class TreeCache implements Closeable
     private final boolean cacheData;
     private final boolean dataIsCompressed;
     private final ListenerContainer<TreeCacheListener> listeners = new ListenerContainer<TreeCacheListener>();
+    private final ListenerContainer<UnhandledErrorListener> errorListeners = new ListenerContainer<UnhandledErrorListener>();
     private final AtomicReference<TreeState> treeState = new AtomicReference<TreeState>(TreeState.LATENT);
 
     private final ConnectionStateListener connectionStateListener = new ConnectionStateListener()
@@ -462,16 +472,16 @@ public class TreeCache implements Closeable
 
     /**
      * Create a TreeCache for the given client and path with default options.
-     *
+     * <p/>
      * If the client is namespaced, all operations on the resulting TreeCache will be in
terms of
      * the namespace, including all published events.  The given path is the root at which
the
      * TreeCache will watch and explore.  If no node exists at the given path, the TreeCache
will
      * be initially empty.
      *
-     * @see #newBuilder(CuratorFramework, String)
      * @param client the client to use; may be namespaced
-     * @param path the path to the root node to watch/explore; this path need not actually
exist on
-     *             the server
+     * @param path   the path to the root node to watch/explore; this path need not actually
exist on
+     *               the server
+     * @see #newBuilder(CuratorFramework, String)
      */
     public TreeCache(CuratorFramework client, String path)
     {
@@ -503,7 +513,7 @@ public class TreeCache implements Closeable
     {
         Preconditions.checkState(treeState.compareAndSet(TreeState.LATENT, TreeState.STARTED),
"already started");
         client.getConnectionStateListenable().addListener(connectionStateListener);
-        if (client.getZookeeperClient().isConnected())
+        if ( client.getZookeeperClient().isConnected() )
         {
             root.wasCreated();
         }
@@ -536,11 +546,16 @@ public class TreeCache implements Closeable
      *
      * @return listenable
      */
-    public ListenerContainer<TreeCacheListener> getListenable()
+    public Listenable<TreeCacheListener> getListenable()
     {
         return listeners;
     }
 
+    public Listenable<UnhandledErrorListener> getUnhandledErrorListenable()
+    {
+        return errorListeners;
+    }
+
     private TreeNode find(String fullPath)
     {
         if ( !fullPath.startsWith(root.path) )
@@ -655,13 +670,33 @@ public class TreeCache implements Closeable
     }
 
     /**
-     * Default behavior is just to log the exception
-     *
-     * @param e the exception
+     * Send an exception to any listeners, or else log the error if there are none.
      */
-    protected void handleException(Throwable e)
+    private void handleException(final Throwable e)
     {
-        LOG.error("", e);
+        if ( errorListeners.size() == 0 )
+        {
+            LOG.error("", e);
+        }
+        else
+        {
+            errorListeners.forEach(new Function<UnhandledErrorListener, Void>()
+            {
+                @Override
+                public Void apply(UnhandledErrorListener listener)
+                {
+                    try
+                    {
+                        listener.unhandledError("", e);
+                    }
+                    catch ( Exception e )
+                    {
+                        LOG.error("Exception handling exception", e);
+                    }
+                    return null;
+                }
+            });
+        }
     }
 
     private void handleStateChange(ConnectionState newState)

http://git-wip-us.apache.org/repos/asf/curator/blob/180a1598/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
index 62edf34..69d3c34 100644
--- 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
@@ -25,13 +25,11 @@ 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.CloseableExecutorService;
 import org.apache.curator.utils.CloseableUtils;
 import org.testng.Assert;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.Executors;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -39,19 +37,22 @@ import java.util.concurrent.atomic.AtomicBoolean;
 public class BaseTestTreeCache extends BaseClassForTests
 {
     CuratorFramework client;
-    MyTreeCache cache;
+    TreeCache cache;
     private final AtomicBoolean hadBackgroundException = new AtomicBoolean(false);
     private final BlockingQueue<TreeCacheEvent> events = new LinkedBlockingQueue<TreeCacheEvent>();
     private final Timing timing = new Timing();
 
+    /**
+     * Automatically records all events into an easily testable event stream.
+     */
     final TreeCacheListener eventListener = new TreeCacheListener()
     {
         @Override
         public void childEvent(CuratorFramework client, TreeCacheEvent event) throws Exception
         {
+            // Suppress any events related to /zookeeper paths
             if ( event.getData() != null && event.getData().getPath().startsWith("/zookeeper")
)
             {
-                // Suppress any events related to /zookeeper paths
                 return;
             }
             events.add(event);
@@ -59,27 +60,38 @@ public class BaseTestTreeCache extends BaseClassForTests
     };
 
     /**
-     * A TreeCache that records exceptions and automatically adds a listener.
+     * Ensures that tests don't cause any background errors.
      */
-    class MyTreeCache extends TreeCache
+    final UnhandledErrorListener errorListener = new UnhandledErrorListener()
     {
-
-        MyTreeCache(CuratorFramework client, String path)
+        @Override
+        public void unhandledError(String message, Throwable e)
         {
-            this(client, path, true);
+            hadBackgroundException.set(true);
+            e.printStackTrace(System.err);
         }
+    };
 
-        MyTreeCache(CuratorFramework client, String path, boolean cacheData)
-        {
-            super(client, path, cacheData, false, new CloseableExecutorService(Executors.newSingleThreadExecutor(TreeCache.defaultThreadFactory),
true));
-            getListenable().addListener(eventListener);
-        }
+    /**
+     * Construct a TreeCache that records exceptions and automatically listens.
+     */
+    protected TreeCache newTreeCacheWithListeners(CuratorFramework client, String path)
+    {
+        TreeCache result = new TreeCache(client, path);
+        result.getListenable().addListener(eventListener);
+        result.getUnhandledErrorListenable().addListener(errorListener);
+        return result;
+    }
 
-        @Override
-        protected void handleException(Throwable e)
-        {
-            handleBackgroundException(e);
-        }
+    /**
+     * Finish constructing a TreeCache that records exceptions and automatically listens.
+     */
+    protected TreeCache buildWithListeners(TreeCache.Builder builder)
+    {
+        TreeCache result = builder.build();
+        result.getListenable().addListener(eventListener);
+        result.getUnhandledErrorListenable().addListener(errorListener);
+        return result;
     }
 
     @Override
@@ -94,20 +106,7 @@ public class BaseTestTreeCache extends BaseClassForTests
     {
         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)
-            {
-                handleBackgroundException(e);
-            }
-        });
-    }
-
-    private void handleBackgroundException(Throwable exception)
-    {
-        hadBackgroundException.set(true);
-        exception.printStackTrace(System.err);
+        client.getUnhandledErrorListenable().addListener(errorListener);
     }
 
     @Override
@@ -132,22 +131,34 @@ public class BaseTestTreeCache extends BaseClassForTests
         }
     }
 
+    /**
+     * Asserts the event queue is empty.
+     */
     void assertNoMoreEvents() throws InterruptedException
     {
         timing.sleepABit();
         Assert.assertTrue(events.isEmpty());
     }
 
+    /**
+     * Asserts the given event is next in the queue, and consumes it from the queue.
+     */
     TreeCacheEvent assertEvent(TreeCacheEvent.Type expectedType) throws InterruptedException
     {
         return assertEvent(expectedType, null);
     }
 
+    /**
+     * Asserts the given event is next in the queue, and consumes it from the queue.
+     */
     TreeCacheEvent assertEvent(TreeCacheEvent.Type expectedType, String expectedPath) throws
InterruptedException
     {
         return assertEvent(expectedType, expectedPath, null);
     }
 
+    /**
+     * Asserts the given event is next in the queue, and consumes it from the queue.
+     */
     TreeCacheEvent assertEvent(TreeCacheEvent.Type expectedType, String expectedPath, byte[]
expectedData) throws InterruptedException
     {
         TreeCacheEvent event = events.poll(timing.forWaiting().seconds(), TimeUnit.SECONDS);

http://git-wip-us.apache.org/repos/asf/curator/blob/180a1598/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 4528751..4f3d914 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
@@ -39,7 +39,7 @@ public class TestTreeCache extends BaseTestTreeCache
         client.create().forPath("/test/3", "three".getBytes());
         client.create().forPath("/test/2/sub", "two-sub".getBytes());
 
-        cache = new MyTreeCache(client, "/test");
+        cache = newTreeCacheWithListeners(client, "/test");
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test");
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/1", "one".getBytes());
@@ -58,7 +58,7 @@ public class TestTreeCache extends BaseTestTreeCache
     @Test
     public void testStartEmpty() throws Exception
     {
-        cache = new MyTreeCache(client, "/test");
+        cache = newTreeCacheWithListeners(client, "/test");
         cache.start();
         assertEvent(TreeCacheEvent.Type.INITIALIZED);
 
@@ -73,7 +73,7 @@ public class TestTreeCache extends BaseTestTreeCache
         client.create().forPath("/test");
         client.create().forPath("/test/one", "hey there".getBytes());
 
-        cache = new MyTreeCache(client, "/test");
+        cache = newTreeCacheWithListeners(client, "/test");
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test");
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/one");
@@ -87,7 +87,7 @@ public class TestTreeCache extends BaseTestTreeCache
         client.create().forPath("/test");
         client.create().forPath("/test/one", "hey there".getBytes());
 
-        cache = new MyTreeCache(client, "/");
+        cache = newTreeCacheWithListeners(client, "/");
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/");
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test");
@@ -109,7 +109,7 @@ public class TestTreeCache extends BaseTestTreeCache
         client.create().forPath("/outer/test");
         client.create().forPath("/outer/test/one", "hey there".getBytes());
 
-        cache = new MyTreeCache(client.usingNamespace("outer"), "/test");
+        cache = newTreeCacheWithListeners(client.usingNamespace("outer"), "/test");
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test");
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/one");
@@ -129,7 +129,7 @@ public class TestTreeCache extends BaseTestTreeCache
         client.create().forPath("/outer/test");
         client.create().forPath("/outer/test/one", "hey there".getBytes());
 
-        cache = new MyTreeCache(client.usingNamespace("outer"), "/");
+        cache = newTreeCacheWithListeners(client.usingNamespace("outer"), "/");
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/");
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/foo");
@@ -147,7 +147,7 @@ public class TestTreeCache extends BaseTestTreeCache
     @Test
     public void testSyncInitialPopulation() throws Exception
     {
-        cache = new MyTreeCache(client, "/test");
+        cache = newTreeCacheWithListeners(client, "/test");
         cache.start();
         assertEvent(TreeCacheEvent.Type.INITIALIZED);
 
@@ -166,7 +166,7 @@ public class TestTreeCache extends BaseTestTreeCache
         client.create().forPath("/test/2", "2".getBytes());
         client.create().forPath("/test/3", "3".getBytes());
 
-        cache = new MyTreeCache(client, "/test");
+        cache = newTreeCacheWithListeners(client, "/test");
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test");
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/1");
@@ -181,7 +181,7 @@ public class TestTreeCache extends BaseTestTreeCache
     {
         client.create().forPath("/test");
 
-        cache = new MyTreeCache(client, "/test", false);
+        cache = buildWithListeners(TreeCache.newBuilder(client, "/test").setCacheData(false));
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test");
         assertEvent(TreeCacheEvent.Type.INITIALIZED);
@@ -192,6 +192,10 @@ public class TestTreeCache extends BaseTestTreeCache
         client.setData().forPath("/test/foo", "something new".getBytes());
         assertEvent(TreeCacheEvent.Type.NODE_UPDATED, "/test/foo");
         assertNoMoreEvents();
+
+        Assert.assertNotNull(cache.getCurrentData("/test/foo"));
+        // No byte data querying the tree because we're not caching data.
+        Assert.assertNull(cache.getCurrentData("/test/foo").getData());
     }
 
     @Test
@@ -200,7 +204,7 @@ public class TestTreeCache extends BaseTestTreeCache
         client.create().forPath("/test");
         client.create().forPath("/test/foo", "one".getBytes());
 
-        cache = new MyTreeCache(client, "/test");
+        cache = newTreeCacheWithListeners(client, "/test");
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test");
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/foo");
@@ -219,7 +223,7 @@ public class TestTreeCache extends BaseTestTreeCache
     {
         client.create().forPath("/test");
 
-        cache = new MyTreeCache(client, "/test");
+        cache = newTreeCacheWithListeners(client, "/test");
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test");
         assertEvent(TreeCacheEvent.Type.INITIALIZED);
@@ -243,7 +247,7 @@ public class TestTreeCache extends BaseTestTreeCache
     {
         client.create().forPath("/test");
 
-        cache = new MyTreeCache(client, "/test");
+        cache = newTreeCacheWithListeners(client, "/test");
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test");
         assertEvent(TreeCacheEvent.Type.INITIALIZED);
@@ -269,7 +273,7 @@ public class TestTreeCache extends BaseTestTreeCache
     @Test
     public void testBasicsOnTwoCaches() throws Exception
     {
-        MyTreeCache cache2 = new MyTreeCache(client, "/test");
+        TreeCache cache2 = newTreeCacheWithListeners(client, "/test");
         cache2.getListenable().removeListener(eventListener);  // Don't listen on the second
cache.
 
         // Just ensures the same event count; enables test flow control on cache2.
@@ -287,7 +291,7 @@ public class TestTreeCache extends BaseTestTreeCache
         {
             client.create().forPath("/test");
 
-            cache = new MyTreeCache(client, "/test");
+            cache = newTreeCacheWithListeners(client, "/test");
             cache.start();
             cache2.start();
 
@@ -327,7 +331,7 @@ public class TestTreeCache extends BaseTestTreeCache
     {
         client.create().forPath("/test");
 
-        cache = new MyTreeCache(client, "/test");
+        cache = newTreeCacheWithListeners(client, "/test");
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test");
         assertEvent(TreeCacheEvent.Type.INITIALIZED);
@@ -357,7 +361,7 @@ public class TestTreeCache extends BaseTestTreeCache
         initCuratorFramework();
 
         // Start the client disconnected.
-        cache = new MyTreeCache(client, "/test");
+        cache = newTreeCacheWithListeners(client, "/test");
         cache.start();
         assertNoMoreEvents();
 

http://git-wip-us.apache.org/repos/asf/curator/blob/180a1598/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
index 0709330..c4501af 100644
--- 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
@@ -63,7 +63,7 @@ public class TestTreeCacheRandomTree extends BaseTestTreeCache
     {
         client.create().forPath("/tree", null);
         CuratorFramework cl = client.usingNamespace("tree");
-        cache = new MyTreeCache(cl, "/");
+        cache = newTreeCacheWithListeners(cl, "/");
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/");
         assertEvent(TreeCacheEvent.Type.INITIALIZED);
@@ -169,7 +169,7 @@ public class TestTreeCacheRandomTree extends BaseTestTreeCache
     /**
      * Recursively assert that current children equal expected children.
      */
-    private static void assertTreeEquals(MyTreeCache cache, TestNode expectedNode)
+    private static void assertTreeEquals(TreeCache cache, TestNode expectedNode)
     {
         String path = expectedNode.fullPath;
         Map<String, ChildData> cacheChildren = cache.getCurrentChildren(path);


Mime
View raw message