You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by dr...@apache.org on 2014/10/07 01:52:26 UTC
[3/6] git commit: Fix formatting; remove MyTreeCache; error listener
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 <sc...@squareup.com>
Authored: Tue Aug 26 19:51:29 2014 -0400
Committer: Scott Blum <sc...@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);