You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ca...@apache.org on 2018/12/10 03:46:33 UTC

[2/8] curator git commit: address review feedback

address review feedback


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

Branch: refs/heads/CURATOR-477
Commit: 88df79b474c252ddb97096c6e967daa38a02b545
Parents: d4a0d95
Author: Rama <ra...@salesforce.com>
Authored: Sun Sep 23 13:28:23 2018 +0530
Committer: Rama <ra...@salesforce.com>
Committed: Sun Sep 23 13:28:23 2018 +0530

----------------------------------------------------------------------
 .../framework/recipes/cache/TreeCache.java      | 90 +++++++++++---------
 .../framework/recipes/cache/TestTreeCache.java  |  2 +-
 2 files changed, 51 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/88df79b4/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 7bcc8d1..7c68868 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
@@ -19,15 +19,35 @@
 
 package org.apache.curator.framework.recipes.cache;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.curator.utils.PathUtils.validatePath;
+
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
+import java.io.Closeable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.WatcherRemoveCuratorFramework;
 import org.apache.curator.framework.api.BackgroundCallback;
 import org.apache.curator.framework.api.CuratorEvent;
+import org.apache.curator.framework.api.ErrorListenerPathable;
+import org.apache.curator.framework.api.GetDataBuilder;
+import org.apache.curator.framework.api.GetDataWatchBackgroundStatable;
 import org.apache.curator.framework.api.UnhandledErrorListener;
 import org.apache.curator.framework.listen.Listenable;
 import org.apache.curator.framework.listen.ListenerContainer;
@@ -42,23 +62,6 @@ import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.data.Stat;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import java.io.Closeable;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.atomic.AtomicReference;
-import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
-
-import static com.google.common.base.Preconditions.checkNotNull;
-import static org.apache.curator.utils.PathUtils.validatePath;
 
 /**
  * <p>A utility that attempts to keep all data from all children of a ZK path locally cached. This class
@@ -73,7 +76,7 @@ public class TreeCache implements Closeable
 {
     private static final Logger LOG = LoggerFactory.getLogger(TreeCache.class);
     private final boolean createParentNodes;
-    private final boolean createZkWatches;
+    private final boolean disableZkWatches;
     private final TreeCacheSelector selector;
 
     public static final class Builder
@@ -85,7 +88,7 @@ public class TreeCache implements Closeable
         private ExecutorService executorService = null;
         private int maxDepth = Integer.MAX_VALUE;
         private boolean createParentNodes = false;
-        private boolean createZkWatches = true;
+        private boolean disableZkWatches = false;
         private TreeCacheSelector selector = new DefaultTreeCacheSelector();
 
         private Builder(CuratorFramework client, String path)
@@ -104,7 +107,7 @@ public class TreeCache implements Closeable
             {
                 executor = Executors.newSingleThreadExecutor(defaultThreadFactory);
             }
-            return new TreeCache(client, path, cacheData, dataIsCompressed, maxDepth, executor, createParentNodes, createZkWatches, selector);
+            return new TreeCache(client, path, cacheData, dataIsCompressed, maxDepth, executor, createParentNodes, disableZkWatches, selector);
         }
 
         /**
@@ -170,12 +173,12 @@ public class TreeCache implements Closeable
         /**
          * By default, TreeCache creates {@link org.apache.zookeeper.ZooKeeper} watches for every created path.
          * Change this behavior with this method.
-         * @param createZkWatches true to create watches
+         * @param disableZkWatches false to create watches
          * @return this for chaining
          */
-        public Builder setCreateZkWatches(boolean createZkWatches)
+        public Builder disableZkWatches(boolean disableZkWatches)
         {
-            this.createZkWatches = createZkWatches;
+            this.disableZkWatches = disableZkWatches;
             return this;
         }
 
@@ -267,10 +270,10 @@ public class TreeCache implements Closeable
         {
             if ( treeState.get() == TreeState.STARTED )
             {
-                if (createZkWatches) {
-                    client.getChildren().usingWatcher(this).inBackground(this).forPath(path);
-                } else {
+                if (disableZkWatches) {
                     client.getChildren().inBackground(this).forPath(path);
+                } else {
+                    client.getChildren().usingWatcher(this).inBackground(this).forPath(path);
                 }
             }
         }
@@ -281,24 +284,31 @@ public class TreeCache implements Closeable
             {
                 if ( dataIsCompressed )
                 {
-                    if (createZkWatches) {
-                        client.getData().decompressed().usingWatcher(this).inBackground(this).forPath(path);
-                    } else {
-                        client.getData().decompressed().inBackground(this).forPath(path);
-                    }
+                    maybeWatch(client.getData().decompressed()).forPath(path);
                 }
                 else
                 {
-                   if (createZkWatches) {
-                       client.getData().usingWatcher(this).inBackground(this).forPath(path);
-                   } else {
-                       client.getData().inBackground(this).forPath(path);
-
-                   }
+                    maybeWatch(client.getData()).forPath(path);
                 }
             }
         }
 
+        private ErrorListenerPathable<byte[]> maybeWatch(GetDataWatchBackgroundStatable dataBuilder) {
+            if (disableZkWatches) {
+                return dataBuilder.inBackground(this);
+            } else {
+                return dataBuilder.usingWatcher(this).inBackground(this);
+            }
+        }
+
+        private ErrorListenerPathable<byte[]> maybeWatch(GetDataBuilder dataBuilder) {
+            if (disableZkWatches) {
+                return dataBuilder.inBackground(this);
+            } else {
+                return dataBuilder.usingWatcher(this).inBackground(this);
+            }
+        }
+
         void wasReconnected() throws Exception
         {
             refresh();
@@ -572,10 +582,10 @@ public class TreeCache implements Closeable
      * @param dataIsCompressed if true, data in the path is compressed
      * @param executorService  Closeable ExecutorService to use for the TreeCache's background thread
      * @param createParentNodes true to create parent nodes as containers
-     * @param createZkWatches true to create Zookeeper watches
+     * @param disableZkWatches false to create Zookeeper watches
      * @param selector         the selector to use
      */
-    TreeCache(CuratorFramework client, String path, boolean cacheData, boolean dataIsCompressed, int maxDepth, final ExecutorService executorService, boolean createParentNodes, boolean createZkWatches, TreeCacheSelector selector)
+    TreeCache(CuratorFramework client, String path, boolean cacheData, boolean dataIsCompressed, int maxDepth, final ExecutorService executorService, boolean createParentNodes, boolean disableZkWatches, TreeCacheSelector selector)
     {
         this.createParentNodes = createParentNodes;
         this.selector = Preconditions.checkNotNull(selector, "selector cannot be null");
@@ -585,7 +595,7 @@ public class TreeCache implements Closeable
         this.cacheData = cacheData;
         this.dataIsCompressed = dataIsCompressed;
         this.maxDepth = maxDepth;
-        this.createZkWatches = createZkWatches;
+        this.disableZkWatches = disableZkWatches;
         this.executorService = Preconditions.checkNotNull(executorService, "executorService cannot be null");
     }
 

http://git-wip-us.apache.org/repos/asf/curator/blob/88df79b4/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 ae15314..762bdd8 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
@@ -474,7 +474,7 @@ public class TestTreeCache extends BaseTestTreeCache
         client.create().forPath("/test/one", "hey there".getBytes());
 
 
-        cache = buildWithListeners(TreeCache.newBuilder(client, "/test").setCreateZkWatches(false));
+        cache = buildWithListeners(TreeCache.newBuilder(client, "/test").disableZkWatches(true));
 
         cache.start();
         assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test");