CURATOR-718: Refactor CuratorFramework inheritance hierarchy by compo…

…sing functionalities Currently, the `CuratorFrameworkImpl` hierarchy is neither simple nor composable. Ideally, there should be only one instance of `CuratorFrameworkImpl`. Additional functionalities should be added by intercepting methods on purpose, but not by cloning through `CuratorFrameworkImpl(CuratorFrameworkImpl parent)`. We could take CURATOR-626 and CURATOR-710 as lessons to know how brittle the currently hierarchy.
apache · Jan 19, 2025 · 288799c · 288799c
1 parent fb78e23
commit 288799c
Show file tree

Hide file tree

Showing 54 changed files with 685 additions and 595 deletions.
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/AddWatchBuilderImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/AddWatchBuilderImpl.java
@@ -34,18 +34,18 @@
 import org.apache.zookeeper.Watcher;
 
 public class AddWatchBuilderImpl implements AddWatchBuilder, Pathable<Void>, BackgroundOperation<String> {
-    private final CuratorFrameworkImpl client;
+    private final InternalCuratorFramework client;
     private Watching watching;
     private Backgrounding backgrounding = new Backgrounding();
     private AddWatchMode mode = AddWatchMode.PERSISTENT_RECURSIVE;
 
-    AddWatchBuilderImpl(CuratorFrameworkImpl client) {
+    AddWatchBuilderImpl(InternalCuratorFramework client) {
         this.client = client;
         watching = new Watching(client, true);
     }
 
     public AddWatchBuilderImpl(
-            CuratorFrameworkImpl client, Watching watching, Backgrounding backgrounding, AddWatchMode mode) {
+            InternalCuratorFramework client, Watching watching, Backgrounding backgrounding, AddWatchMode mode) {
         this.client = client;
         this.watching = watching;
         this.backgrounding = backgrounding;

diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/BackgroundSyncImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/BackgroundSyncImpl.java
@@ -24,10 +24,10 @@
 import org.apache.zookeeper.AsyncCallback;
 
 class BackgroundSyncImpl implements BackgroundOperation<String> {
-    private final CuratorFrameworkImpl client;
+    private final InternalCuratorFramework client;
     private final Object context;
 
-    BackgroundSyncImpl(CuratorFrameworkImpl client, Object context) {
+    BackgroundSyncImpl(InternalCuratorFramework client, Object context) {
         this.client = client;
         this.context = context;
     }

diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/Backgrounding.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/Backgrounding.java
@@ -62,11 +62,11 @@ public class Backgrounding {
         errorListener = null;
     }
 
-    Backgrounding(CuratorFrameworkImpl client, BackgroundCallback callback, Object context, Executor executor) {
+    Backgrounding(InternalCuratorFramework client, BackgroundCallback callback, Object context, Executor executor) {
         this(wrapCallback(client, callback, executor), context);
     }
 
-    Backgrounding(CuratorFrameworkImpl client, BackgroundCallback callback, Executor executor) {
+    Backgrounding(InternalCuratorFramework client, BackgroundCallback callback, Executor executor) {
         this(wrapCallback(client, callback, executor));
     }
 
@@ -119,7 +119,7 @@ void checkError(Throwable e, Watching watching) throws Exception {
     }
 
     private static BackgroundCallback wrapCallback(
-            final CuratorFrameworkImpl client, final BackgroundCallback callback, final Executor executor) {
+            final InternalCuratorFramework client, final BackgroundCallback callback, final Executor executor) {
         return new BackgroundCallback() {
             @Override
             public void processResult(CuratorFramework dummy, final CuratorEvent event) throws Exception {
@@ -131,7 +131,7 @@ public void run() {
                         } catch (Exception e) {
                             ThreadUtils.checkInterrupted(e);
                             if (e instanceof KeeperException) {
-                                client.validateConnection(client.codeToState(((KeeperException) e).code()));
+                                client.validateConnection(FrameworkUtils.codeToState(((KeeperException) e).code()));
                             }
                             client.logError("Background operation result handling threw exception", e);
                         }

diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
@@ -70,7 +70,7 @@ public class CreateBuilderImpl
                 BackgroundOperation<PathAndBytes>,
                 ErrorListenerPathAndBytesable<String> {
     private final Logger log = LoggerFactory.getLogger(getClass());
-    private final CuratorFrameworkImpl client;
+    private final InternalCuratorFramework client;
     private final ProtectedMode protectedMode = new ProtectedMode();
     private CreateMode createMode;
     private Backgrounding backgrounding;
@@ -93,7 +93,7 @@ public class CreateBuilderImpl
     @VisibleForTesting
     boolean failNextIdempotentCheckForTesting = false;
 
-    CreateBuilderImpl(CuratorFrameworkImpl client) {
+    CreateBuilderImpl(InternalCuratorFramework client) {
         this.client = client;
         createMode = CreateMode.PERSISTENT;
         backgrounding = new Backgrounding();
@@ -107,7 +107,7 @@ public class CreateBuilderImpl
     }
 
     public CreateBuilderImpl(
-            CuratorFrameworkImpl client,
+            InternalCuratorFramework client,
             CreateMode createMode,
             Backgrounding backgrounding,
             boolean createParentsIfNeeded,
@@ -737,7 +737,7 @@ public ProtectACLCreateModePathAndBytesable<String> creatingParentContainersIfNe
     }
 
     static <T> void backgroundCreateParentsThenNode(
-            final CuratorFrameworkImpl client,
+            final InternalCuratorFramework client,
             final OperationAndData<T> mainOperationAndData,
             final String path,
             final InternalACLProvider aclProvider,
@@ -766,7 +766,7 @@ public CuratorEventType getBackgroundEventType() {
     }
 
     private void backgroundSetData(
-            final CuratorFrameworkImpl client,
+            final InternalCuratorFramework client,
             final OperationAndData<PathAndBytes> mainOperationAndData,
             final String path,
             final Backgrounding backgrounding) {
@@ -801,7 +801,7 @@ public CuratorEventType getBackgroundEventType() {
     }
 
     private void backgroundCheckIdempotent(
-            final CuratorFrameworkImpl client,
+            final InternalCuratorFramework client,
             final OperationAndData<PathAndBytes> mainOperationAndData,
             final String path,
             final Backgrounding backgrounding) {
@@ -844,7 +844,7 @@ private void sendBackgroundResponse(
     }
 
     private static <T> void sendBackgroundResponse(
-            CuratorFrameworkImpl client,
+            InternalCuratorFramework client,
             int rc,
             String path,
             Object ctx,

diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorEventImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorEventImpl.java
@@ -114,7 +114,7 @@ public String toString() {
     }
 
     CuratorEventImpl(
-            CuratorFrameworkImpl client,
+            InternalCuratorFramework client,
             CuratorEventType type,
             int resultCode,
             String path,