You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@twill.apache.org by hs...@apache.org on 2015/09/25 21:49:29 UTC

incubator-twill git commit: (TWILL-141) Fix namespacing of ZKClient

Repository: incubator-twill
Updated Branches:
  refs/heads/master 66402b4f2 -> f88e18f75


(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 <hs...@apache.org>


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/master
Commit: f88e18f7587aae3528b1e47a22b0b281ff91f95e
Parents: 66402b4
Author: Terence Yim <ch...@apache.org>
Authored: Thu Sep 24 03:22:24 2015 -0700
Committer: hsaputra <hs...@apache.org>
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();
+    }
+  }
 }