fixup! CURATOR-718: Refactor CuratorFramework inheritance hierarchy b…

…y composing functionalities Notable changes to last review: 1. Rename `InternalCuratorFramework` to `CuratorFrameworkBase`. 2. Make `DelegatingCuratorFramework` `abstract` and package `private`. 3. Move `start`/`close` from `DelegatingCuratorFramework` to subclasses.
apache · Feb 7, 2025 · 8c37651 · 8c37651
1 parent c47cc6f
commit 8c37651
Show file tree

Hide file tree

Showing 52 changed files with 170 additions and 161 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 InternalCuratorFramework client;
+    private final CuratorFrameworkBase client;
     private Watching watching;
     private Backgrounding backgrounding = new Backgrounding();
     private AddWatchMode mode = AddWatchMode.PERSISTENT_RECURSIVE;
 
-    AddWatchBuilderImpl(InternalCuratorFramework client) {
+    AddWatchBuilderImpl(CuratorFrameworkBase client) {
         this.client = client;
         watching = new Watching(client, true);
     }
 
     public AddWatchBuilderImpl(
-            InternalCuratorFramework client, Watching watching, Backgrounding backgrounding, AddWatchMode mode) {
+            CuratorFrameworkBase 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 InternalCuratorFramework client;
+    private final CuratorFrameworkBase client;
     private final Object context;
 
-    BackgroundSyncImpl(InternalCuratorFramework client, Object context) {
+    BackgroundSyncImpl(CuratorFrameworkBase 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(InternalCuratorFramework client, BackgroundCallback callback, Object context, Executor executor) {
+    Backgrounding(CuratorFrameworkBase client, BackgroundCallback callback, Object context, Executor executor) {
         this(wrapCallback(client, callback, executor), context);
     }
 
-    Backgrounding(InternalCuratorFramework client, BackgroundCallback callback, Executor executor) {
+    Backgrounding(CuratorFrameworkBase 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 InternalCuratorFramework client, final BackgroundCallback callback, final Executor executor) {
+            final CuratorFrameworkBase client, final BackgroundCallback callback, final Executor executor) {
         return new BackgroundCallback() {
             @Override
             public void processResult(CuratorFramework dummy, final CuratorEvent event) throws Exception {

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 InternalCuratorFramework client;
+    private final CuratorFrameworkBase 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(InternalCuratorFramework client) {
+    CreateBuilderImpl(CuratorFrameworkBase client) {
         this.client = client;
         createMode = CreateMode.PERSISTENT;
         backgrounding = new Backgrounding();
@@ -107,7 +107,7 @@ public class CreateBuilderImpl
     }
 
     public CreateBuilderImpl(
-            InternalCuratorFramework client,
+            CuratorFrameworkBase client,
             CreateMode createMode,
             Backgrounding backgrounding,
             boolean createParentsIfNeeded,
@@ -737,7 +737,7 @@ public ProtectACLCreateModePathAndBytesable<String> creatingParentContainersIfNe
     }
 
     static <T> void backgroundCreateParentsThenNode(
-            final InternalCuratorFramework client,
+            final CuratorFrameworkBase client,
             final OperationAndData<T> mainOperationAndData,
             final String path,
             final InternalACLProvider aclProvider,
@@ -766,7 +766,7 @@ public CuratorEventType getBackgroundEventType() {
     }
 
     private void backgroundSetData(
-            final InternalCuratorFramework client,
+            final CuratorFrameworkBase client,
             final OperationAndData<PathAndBytes> mainOperationAndData,
             final String path,
             final Backgrounding backgrounding) {
@@ -801,7 +801,7 @@ public CuratorEventType getBackgroundEventType() {
     }
 
     private void backgroundCheckIdempotent(
-            final InternalCuratorFramework client,
+            final CuratorFrameworkBase client,
             final OperationAndData<PathAndBytes> mainOperationAndData,
             final String path,
             final Backgrounding backgrounding) {
@@ -844,7 +844,7 @@ private void sendBackgroundResponse(
     }
 
     private static <T> void sendBackgroundResponse(
-            InternalCuratorFramework client,
+            CuratorFrameworkBase 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(
-            InternalCuratorFramework client,
+            CuratorFrameworkBase client,
             CuratorEventType type,
             int resultCode,
             String path,

diff --git a/...mework/imps/InternalCuratorFramework.java → .../framework/imps/CuratorFrameworkBase.java b/...mework/imps/InternalCuratorFramework.java → .../framework/imps/CuratorFrameworkBase.java
@@ -48,14 +48,14 @@
 import org.apache.zookeeper.ZooKeeper;
 
 /**
- * This is the internal version of {@link CuratorFramework}.
+ * This is the base class of all {@link CuratorFramework}s, it is public for private usages (a.k.a. impls/details package).
  *
- * <p>Most internal codes should use {@link InternalCuratorFramework} instead of {@link CuratorFrameworkImpl}, so
+ * <p>Most internal codes should use {@link CuratorFrameworkBase} instead of {@link CuratorFrameworkImpl}, so
  * functionalities could be added additively by overriding methods in {@link DelegatingCuratorFramework}.
  *
- * <p>An instance of {@link CuratorFramework} MUST BE an instance of {@link InternalCuratorFramework}.
+ * <p>An instance of {@link CuratorFramework} MUST BE an instance of {@link CuratorFrameworkBase}.
  */
-public abstract class InternalCuratorFramework implements CuratorFramework {
+public abstract class CuratorFrameworkBase implements CuratorFramework {
     abstract NamespaceImpl getNamespaceImpl();
 
     @Override
@@ -111,7 +111,7 @@ final ZooKeeper getZooKeeper() throws Exception {
         return getZookeeperClient().getZooKeeper();
     }
 
-    protected final void internalSync(InternalCuratorFramework impl, String path, Object context) {
+    protected final void internalSync(CuratorFrameworkBase impl, String path, Object context) {
         BackgroundOperation<String> operation = new BackgroundSyncImpl(impl, context);
         processBackgroundOperation(new OperationAndData(operation, path, null, null, context, null), null);
     }

diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
@@ -74,7 +74,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public final class CuratorFrameworkImpl extends InternalCuratorFramework {
+public final class CuratorFrameworkImpl extends CuratorFrameworkBase {
     private final Logger log = LoggerFactory.getLogger(getClass());
     private final CuratorZookeeperClient client;
     private final StandardListenerManager<CuratorListener> listeners;

diff --git a/...ramework/src/main/java/org/apache/curator/framework/imps/CuratorMultiTransactionImpl.java b/...ramework/src/main/java/org/apache/curator/framework/imps/CuratorMultiTransactionImpl.java
@@ -50,14 +50,14 @@ public class CuratorMultiTransactionImpl
                 CuratorMultiTransactionMain,
                 BackgroundOperation<CuratorMultiTransactionRecord>,
                 ErrorListenerMultiTransactionMain {
-    private final InternalCuratorFramework client;
+    private final CuratorFrameworkBase client;
     private Backgrounding backgrounding = new Backgrounding();
 
-    public CuratorMultiTransactionImpl(InternalCuratorFramework client) {
+    public CuratorMultiTransactionImpl(CuratorFrameworkBase client) {
         this.client = client;
     }
 
-    public CuratorMultiTransactionImpl(InternalCuratorFramework client, Backgrounding backgrounding) {
+    public CuratorMultiTransactionImpl(CuratorFrameworkBase client, Backgrounding backgrounding) {
         this.client = client;
         this.backgrounding = backgrounding;
     }

diff --git a/...r-framework/src/main/java/org/apache/curator/framework/imps/CuratorTempFrameworkImpl.java b/...r-framework/src/main/java/org/apache/curator/framework/imps/CuratorTempFrameworkImpl.java
@@ -36,7 +36,7 @@ public class CuratorTempFrameworkImpl implements CuratorTempFramework {
     private final long inactiveThresholdMs;
 
     // guarded by sync
-    private InternalCuratorFramework client;
+    private CuratorFrameworkBase client;
 
     // guarded by sync
     private ScheduledExecutorService cleanup;
@@ -67,7 +67,7 @@ public TempGetDataBuilder getData() throws Exception {
     }
 
     @VisibleForTesting
-    synchronized InternalCuratorFramework getClient() {
+    synchronized CuratorFrameworkBase getClient() {
         return client;
     }
 
@@ -83,7 +83,7 @@ synchronized void updateLastAccess() {
 
     private synchronized void openConnectionIfNeeded() throws Exception {
         if (client == null) {
-            client = (InternalCuratorFramework) factory.build(); // cast is safe - we control both sides of this
+            client = (CuratorFrameworkBase) factory.build(); // cast is safe - we control both sides of this
             client.start();
         }
 

diff --git a/...tor-framework/src/main/java/org/apache/curator/framework/imps/CuratorTransactionImpl.java b/...tor-framework/src/main/java/org/apache/curator/framework/imps/CuratorTransactionImpl.java
@@ -35,12 +35,12 @@
 
 @SuppressWarnings("deprecation")
 class CuratorTransactionImpl implements CuratorTransaction, CuratorTransactionBridge, CuratorTransactionFinal {
-    private final InternalCuratorFramework client;
+    private final CuratorFrameworkBase client;
     private final CuratorMultiTransactionRecord transaction;
 
     private boolean isCommitted = false;
 
-    CuratorTransactionImpl(InternalCuratorFramework client) {
+    CuratorTransactionImpl(CuratorFrameworkBase client) {
         this.client = client;
         transaction = new CuratorMultiTransactionRecord();
     }
@@ -83,7 +83,7 @@ public TransactionCheckBuilder<CuratorTransactionBridge> check() {
     }
 
     static <T> TransactionCheckBuilder<T> makeTransactionCheckBuilder(
-            final InternalCuratorFramework client, final T context, final CuratorMultiTransactionRecord transaction) {
+            final CuratorFrameworkBase client, final T context, final CuratorMultiTransactionRecord transaction) {
         return new TransactionCheckBuilder<T>() {
             private int version = -1;
 
@@ -125,7 +125,7 @@ public List<OpResult> call() throws Exception {
     }
 
     static List<CuratorTransactionResult> wrapResults(
-            InternalCuratorFramework client, List<OpResult> resultList, CuratorMultiTransactionRecord transaction) {
+            CuratorFrameworkBase client, List<OpResult> resultList, CuratorMultiTransactionRecord transaction) {
         ImmutableList.Builder<CuratorTransactionResult> builder = ImmutableList.builder();
         for (int i = 0; i < resultList.size(); ++i) {
             OpResult opResult = resultList.get(i);
@@ -138,7 +138,7 @@ static List<CuratorTransactionResult> wrapResults(
     }
 
     static CuratorTransactionResult makeCuratorResult(
-            InternalCuratorFramework client, OpResult opResult, TypeAndPath metadata) {
+            CuratorFrameworkBase client, OpResult opResult, TypeAndPath metadata) {
         String resultPath = null;
         Stat resultStat = null;
         int error = 0;

diff --git a/...framework/src/main/java/org/apache/curator/framework/imps/DelegatingCuratorFramework.java b/...framework/src/main/java/org/apache/curator/framework/imps/DelegatingCuratorFramework.java
@@ -36,23 +36,17 @@
 import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.server.quorum.flexible.QuorumVerifier;
 
-public class DelegatingCuratorFramework extends InternalCuratorFramework {
-    protected final InternalCuratorFramework client;
+/**
+ * Delegates methods to shadowed {@link CuratorFrameworkBase} so subclasses can override only methods that need
+ * additional cares.
+ */
+abstract class DelegatingCuratorFramework extends CuratorFrameworkBase {
+    protected final CuratorFrameworkBase client;
 
-    public DelegatingCuratorFramework(InternalCuratorFramework client) {
+    public DelegatingCuratorFramework(CuratorFrameworkBase client) {
         this.client = client;
     }
 
-    @Override
-    public void start() {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public void close() {
-        throw new UnsupportedOperationException();
-    }
-
     @Override
     public CuratorFrameworkState getState() {
         return client.getState();
@@ -201,8 +195,7 @@ NamespaceFacadeCache getNamespaceFacadeCache() {
     }
 
     @Override
-    public <DATA_TYPE> void processBackgroundOperation(
-            OperationAndData<DATA_TYPE> operationAndData, CuratorEvent event) {
+    <DATA_TYPE> void processBackgroundOperation(OperationAndData<DATA_TYPE> operationAndData, CuratorEvent event) {
         client.processBackgroundOperation(operationAndData, event);
     }
 

diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/DeleteBuilderImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/DeleteBuilderImpl.java
@@ -34,7 +34,7 @@
 import org.apache.zookeeper.Op;
 
 public class DeleteBuilderImpl implements DeleteBuilder, BackgroundOperation<String>, ErrorListenerPathable<Void> {
-    private final InternalCuratorFramework client;
+    private final CuratorFrameworkBase client;
     private int version;
     private Backgrounding backgrounding;
     private boolean deletingChildrenIfNeeded;
@@ -47,7 +47,7 @@ public class DeleteBuilderImpl implements DeleteBuilder, BackgroundOperation<Str
     @VisibleForTesting
     boolean failBeforeNextDeleteForTesting = false;
 
-    DeleteBuilderImpl(InternalCuratorFramework client) {
+    DeleteBuilderImpl(CuratorFrameworkBase client) {
         this.client = client;
         version = -1;
         backgrounding = new Backgrounding();
@@ -57,7 +57,7 @@ public class DeleteBuilderImpl implements DeleteBuilder, BackgroundOperation<Str
     }
 
     public DeleteBuilderImpl(
-            InternalCuratorFramework client,
+            CuratorFrameworkBase client,
             int version,
             Backgrounding backgrounding,
             boolean deletingChildrenIfNeeded,

diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/ExistsBuilderImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/ExistsBuilderImpl.java
@@ -34,19 +34,19 @@
 
 public class ExistsBuilderImpl
         implements ExistsBuilder, BackgroundOperation<String>, ErrorListenerPathable<Stat>, ACLableExistBuilderMain {
-    private final InternalCuratorFramework client;
+    private final CuratorFrameworkBase client;
     private Backgrounding backgrounding;
     private Watching watching;
     private boolean createParentsIfNeeded;
     private boolean createParentContainersIfNeeded;
     private ACLing acling;
 
-    ExistsBuilderImpl(InternalCuratorFramework client) {
+    ExistsBuilderImpl(CuratorFrameworkBase client) {
         this(client, new Backgrounding(), null, false, false);
     }
 
     public ExistsBuilderImpl(
-            InternalCuratorFramework client,
+            CuratorFrameworkBase client,
             Backgrounding backgrounding,
             Watcher watcher,
             boolean createParentsIfNeeded,

diff --git a/...c/main/java/org/apache/curator/framework/imps/FindAndDeleteProtectedNodeInBackground.java b/...c/main/java/org/apache/curator/framework/imps/FindAndDeleteProtectedNodeInBackground.java
@@ -34,12 +34,12 @@
 
 class FindAndDeleteProtectedNodeInBackground implements BackgroundOperation<Void> {
     private final Logger log = LoggerFactory.getLogger(getClass());
-    private final InternalCuratorFramework client;
+    private final CuratorFrameworkBase client;
     private final String namespaceAdjustedParentPath;
     private final String protectedId;
 
     FindAndDeleteProtectedNodeInBackground(
-            InternalCuratorFramework client, String namespaceAdjustedParentPath, String protectedId) {
+            CuratorFrameworkBase client, String namespaceAdjustedParentPath, String protectedId) {
         this.client = client;
         this.namespaceAdjustedParentPath = namespaceAdjustedParentPath;
         this.protectedId = protectedId;

diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/GetACLBuilderImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/GetACLBuilderImpl.java
@@ -35,18 +35,18 @@
 import org.apache.zookeeper.data.Stat;
 
 public class GetACLBuilderImpl implements GetACLBuilder, BackgroundOperation<String>, ErrorListenerPathable<List<ACL>> {
-    private final InternalCuratorFramework client;
+    private final CuratorFrameworkBase client;
 
     private Backgrounding backgrounding;
     private Stat responseStat;
 
-    GetACLBuilderImpl(InternalCuratorFramework client) {
+    GetACLBuilderImpl(CuratorFrameworkBase client) {
         this.client = client;
         backgrounding = new Backgrounding();
         responseStat = new Stat();
     }
 
-    public GetACLBuilderImpl(InternalCuratorFramework client, Backgrounding backgrounding, Stat responseStat) {
+    public GetACLBuilderImpl(CuratorFrameworkBase client, Backgrounding backgrounding, Stat responseStat) {
         this.client = client;
         this.backgrounding = backgrounding;
         this.responseStat = responseStat;