Return-Path: X-Original-To: apmail-curator-commits-archive@minotaur.apache.org Delivered-To: apmail-curator-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id D327C17AD9 for ; Mon, 6 Oct 2014 23:52:24 +0000 (UTC) Received: (qmail 78132 invoked by uid 500); 6 Oct 2014 23:52:24 -0000 Delivered-To: apmail-curator-commits-archive@curator.apache.org Received: (qmail 78053 invoked by uid 500); 6 Oct 2014 23:52:24 -0000 Mailing-List: contact commits-help@curator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@curator.apache.org Delivered-To: mailing list commits@curator.apache.org Received: (qmail 78030 invoked by uid 99); 6 Oct 2014 23:52:24 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 06 Oct 2014 23:52:24 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 6CA4A8B439F; Mon, 6 Oct 2014 23:52:24 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: dragonsinth@apache.org To: commits@curator.apache.org Date: Mon, 06 Oct 2014 23:52:26 -0000 Message-Id: In-Reply-To: <9869c2d0a51f4ce0ae5662b4d5a1c7d6@git.apache.org> References: <9869c2d0a51f4ce0ae5662b4d5a1c7d6@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [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 Authored: Tue Aug 26 19:51:29 2014 -0400 Committer: Scott Blum 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. - * + *

* 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 listeners = new ListenerContainer(); + private final ListenerContainer errorListeners = new ListenerContainer(); private final AtomicReference treeState = new AtomicReference(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. - * + *

* 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 getListenable() + public Listenable getListenable() { return listeners; } + public Listenable 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() + { + @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 events = new LinkedBlockingQueue(); 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 cacheChildren = cache.getCurrentChildren(path);