(TWILL-141) Fix namespacing of ZKClient - Not to fail with exception when creating â/â through the namespaced ZKClient - Return the correct path in OperationFuture.getRequestPath() for futures returned from namespaced ZKClient
This closes #64 from GitHub. Signed-off-by: hsaputra <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-twill/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-twill/commit/f88e18f7 Tree: http://git-wip-us.apache.org/repos/asf/incubator-twill/tree/f88e18f7 Diff: http://git-wip-us.apache.org/repos/asf/incubator-twill/diff/f88e18f7 Branch: refs/heads/site Commit: f88e18f7587aae3528b1e47a22b0b281ff91f95e Parents: 66402b4 Author: Terence Yim <[email protected]> Authored: Thu Sep 24 03:22:24 2015 -0700 Committer: hsaputra <[email protected]> Committed: Fri Sep 25 12:17:53 2015 -0700 ---------------------------------------------------------------------- .../internal/zookeeper/NamespaceZKClient.java | 33 ++++++++---- .../zookeeper/SettableOperationFuture.java | 2 +- .../apache/twill/zookeeper/ZKClientTest.java | 55 ++++++++++++++++++++ 3 files changed, 78 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-twill/blob/f88e18f7/twill-zookeeper/src/main/java/org/apache/twill/internal/zookeeper/NamespaceZKClient.java ---------------------------------------------------------------------- diff --git a/twill-zookeeper/src/main/java/org/apache/twill/internal/zookeeper/NamespaceZKClient.java b/twill-zookeeper/src/main/java/org/apache/twill/internal/zookeeper/NamespaceZKClient.java index e19bb0a..239a656 100644 --- a/twill-zookeeper/src/main/java/org/apache/twill/internal/zookeeper/NamespaceZKClient.java +++ b/twill-zookeeper/src/main/java/org/apache/twill/internal/zookeeper/NamespaceZKClient.java @@ -70,47 +70,58 @@ public final class NamespaceZKClient extends ForwardingZKClient { @Override public OperationFuture<String> create(String path, @Nullable byte[] data, CreateMode createMode, boolean createParent, Iterable<ACL> acl) { - return relayPath(delegate.create(namespace + path, data, createMode, createParent, acl), + return relayPath(delegate.create(getNamespacedPath(path), data, createMode, createParent, acl), this.<String>createFuture(path)); } @Override public OperationFuture<Stat> exists(String path, @Nullable Watcher watcher) { - return relayFuture(delegate.exists(namespace + path, watcher), this.<Stat>createFuture(path)); + return relayFuture(delegate.exists(getNamespacedPath(path), watcher), this.<Stat>createFuture(path)); } @Override public OperationFuture<NodeChildren> getChildren(String path, @Nullable Watcher watcher) { - return relayFuture(delegate.getChildren(namespace + path, watcher), this.<NodeChildren>createFuture(path)); + return relayFuture(delegate.getChildren(getNamespacedPath(path), watcher), this.<NodeChildren>createFuture(path)); } @Override public OperationFuture<NodeData> getData(String path, @Nullable Watcher watcher) { - return relayFuture(delegate.getData(namespace + path, watcher), this.<NodeData>createFuture(path)); + return relayFuture(delegate.getData(getNamespacedPath(path), watcher), this.<NodeData>createFuture(path)); } @Override public OperationFuture<Stat> setData(String dataPath, byte[] data, int version) { - return relayFuture(delegate.setData(namespace + dataPath, data, version), this.<Stat>createFuture(dataPath)); + return relayFuture(delegate.setData(getNamespacedPath(dataPath), data, version), this.<Stat>createFuture(dataPath)); } @Override public OperationFuture<String> delete(String deletePath, int version) { - return relayPath(delegate.delete(namespace + deletePath, version), this.<String>createFuture(deletePath)); + return relayPath(delegate.delete(getNamespacedPath(deletePath), version), this.<String>createFuture(deletePath)); } @Override public OperationFuture<ACLData> getACL(String path) { - return relayFuture(delegate.getACL(namespace + path), this.<ACLData>createFuture(path)); + return relayFuture(delegate.getACL(getNamespacedPath(path)), this.<ACLData>createFuture(path)); } @Override public OperationFuture<Stat> setACL(String path, Iterable<ACL> acl, int version) { - return relayFuture(delegate.setACL(namespace + path, acl, version), this.<Stat>createFuture(path)); + return relayFuture(delegate.setACL(getNamespacedPath(path), acl, version), this.<Stat>createFuture(path)); + } + + /** + * Returns the namespaced path for the given path. The returned path should be used when performing + * ZK operations with the delegating ZKClient. + */ + private String getNamespacedPath(String path) { + if ("/".equals(path)) { + return namespace; + } + return namespace + path; } private <V> SettableOperationFuture<V> createFuture(String path) { - return SettableOperationFuture.create(namespace + path, Threads.SAME_THREAD_EXECUTOR); + return SettableOperationFuture.create(path, Threads.SAME_THREAD_EXECUTOR); } private <V> OperationFuture<V> relayFuture(final OperationFuture<V> from, final SettableOperationFuture<V> to) { @@ -134,8 +145,8 @@ public final class NamespaceZKClient extends ForwardingZKClient { @Override public void run() { try { - String path = from.get(); - to.set(path.substring(namespace.length())); + String relativePath = from.get().substring(namespace.length()); + to.set(relativePath.isEmpty() ? "/" : relativePath); } catch (Exception e) { to.setException(e.getCause()); } http://git-wip-us.apache.org/repos/asf/incubator-twill/blob/f88e18f7/twill-zookeeper/src/main/java/org/apache/twill/internal/zookeeper/SettableOperationFuture.java ---------------------------------------------------------------------- diff --git a/twill-zookeeper/src/main/java/org/apache/twill/internal/zookeeper/SettableOperationFuture.java b/twill-zookeeper/src/main/java/org/apache/twill/internal/zookeeper/SettableOperationFuture.java index 06f089e..f98b8f6 100644 --- a/twill-zookeeper/src/main/java/org/apache/twill/internal/zookeeper/SettableOperationFuture.java +++ b/twill-zookeeper/src/main/java/org/apache/twill/internal/zookeeper/SettableOperationFuture.java @@ -35,7 +35,7 @@ public final class SettableOperationFuture<V> extends AbstractFuture<V> implemen private final Executor executor; public static <V> SettableOperationFuture<V> create(String path, Executor executor) { - return new SettableOperationFuture<V>(path, executor); + return new SettableOperationFuture<>(path, executor); } private SettableOperationFuture(String requestPath, Executor executor) { http://git-wip-us.apache.org/repos/asf/incubator-twill/blob/f88e18f7/twill-zookeeper/src/test/java/org/apache/twill/zookeeper/ZKClientTest.java ---------------------------------------------------------------------- diff --git a/twill-zookeeper/src/test/java/org/apache/twill/zookeeper/ZKClientTest.java b/twill-zookeeper/src/test/java/org/apache/twill/zookeeper/ZKClientTest.java index a9120c3..b9cb8a4 100644 --- a/twill-zookeeper/src/test/java/org/apache/twill/zookeeper/ZKClientTest.java +++ b/twill-zookeeper/src/test/java/org/apache/twill/zookeeper/ZKClientTest.java @@ -394,4 +394,59 @@ public class ZKClientTest { serverThread.interrupt(); } } + + @Test + public void testNamespace() throws ExecutionException, InterruptedException { + InMemoryZKServer zkServer = InMemoryZKServer.builder().setTickTime(1000).build(); + zkServer.startAndWait(); + + try { + ZKClientService zkClient = ZKClientService.Builder + .of(zkServer.getConnectionStr()) + .build(); + zkClient.startAndWait(); + + ZKClient zk = ZKClients.namespace(zkClient, "/test"); + // Create the "/ should create the "/test" from the root + OperationFuture<String> createFuture = zk.create("/", null, CreateMode.PERSISTENT); + // Shouldn't have namespace as prefix for path returned from the future. + Assert.assertEquals("/", createFuture.getRequestPath()); + Assert.assertEquals("/", createFuture.get()); + + // Create a path under the namespace + createFuture = zk.create("/subpath", null, CreateMode.PERSISTENT); + Assert.assertEquals("/subpath", createFuture.getRequestPath()); + Assert.assertEquals("/subpath", createFuture.get()); + + // Check for exists + OperationFuture<Stat> existsFuture = zk.exists("/subpath"); + Assert.assertEquals("/subpath", existsFuture.getRequestPath()); + Assert.assertNotNull(existsFuture.get()); + + // Put some data + OperationFuture<Stat> setFuture = zk.setData("/subpath", "hello".getBytes()); + Assert.assertEquals("/subpath", setFuture.getRequestPath()); + Assert.assertNotNull(setFuture.get()); + + // Read the data back + OperationFuture<NodeData> getFuture = zk.getData("/subpath"); + Assert.assertEquals("/subpath", getFuture.getRequestPath()); + Assert.assertArrayEquals("hello".getBytes(), getFuture.get().getData()); + + // Delete the sub path + OperationFuture < String > deleteFuture = zk.delete("/subpath"); + Assert.assertEquals("/subpath", deleteFuture.getRequestPath()); + Assert.assertEquals("/subpath", deleteFuture.get()); + + // Delete the namespace root + deleteFuture = zk.delete("/"); + Assert.assertEquals("/", deleteFuture.getRequestPath()); + Assert.assertEquals("/", deleteFuture.get()); + + // The namespace must be gone + Assert.assertNull(zkClient.exists("/test").get()); + } finally { + zkServer.stopAndWait(); + } + } }
