Changes based on PR comments

helix-core/src/main/java/org/apache/helix/manager/zk/ZkBucketDataAccessor.java

@@ -356,7 +356,7 @@ public void finalize() {
     close();
   }
 
-  int getAccessOption(long znodeTTLms) {
+  static int getAccessOption(long znodeTTLms) {
     if(znodeTTLms > 0) {
       return AccessOption.PERSISTENT_WITH_TTL;
     } else {

helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBaseDataAccessor.java

@@ -165,6 +165,7 @@ record = new ZNRecord("msg_1");
    */
   @Test
   public void testAsyncSetChildrenWithTTL() {
+    // Step 1: Enable extended types in Zookeeper for TTL support
     System.setProperty("zookeeper.extendedTypesEnabled", "true");
     String className = TestHelper.getTestClassName();
     String methodName = TestHelper.getTestMethodName();
@@ -178,6 +179,7 @@ public void testAsyncSetChildrenWithTTL() {
     List<String> paths = new ArrayList<>();
     ZkBaseDataAccessor<ZNRecord> accessor = Mockito.spy(new ZkBaseDataAccessor<ZNRecord>(_gZkClient));
 
+    // Step 2: Create 10 ZNRecord objects and corresponding paths
     for (int i = 0; i < 10; i++) {
       String msgId = "msg_" + i;
       // Example path: /TestZkBaseDataAccessor/INSTANCES/host_1/MESSAGES/msg_id
@@ -186,16 +188,19 @@ public void testAsyncSetChildrenWithTTL() {
       newRecord.setSimpleField("key1", "value1");
       records.add(newRecord);
     }
+
+    // Step 3: Set the 10 ZNRecord objects with TTL
     boolean[] success = accessor.setChildren(paths, records, AccessOption.PERSISTENT_WITH_TTL, ttl);
     for (int i = 0; i < 10; i++) {
       String msgId = "msg_" + i;
       Assert.assertTrue(success[i], "Should succeed in set " + msgId);
     }
 
-    // Verify if all 5 subpaths to be created are configured with TTL
+    // Step 4: Verify if all 5 subpaths to be created are configured with TTL
     Mockito.verify(accessor, Mockito.times(5)).create(Mockito.any(),
         Mockito.any(), Mockito.any(), Mockito.any(), Mockito.eq(AccessOption.PERSISTENT_WITH_TTL), Mockito.eq(ttl));
 
+    // Step 5: Clear the extended types property
     System.clearProperty("zookeeper.extendedTypesEnabled");
     System.out.println("END " + testName + " at " + new Date(System.currentTimeMillis()));
   }
@@ -467,6 +472,7 @@ public ZNRecord update(ZNRecord currentData) {
    */
   @Test
   public void testSyncDoUpdateWithTTL() {
+    // Step 1: Enable extended types in Zookeeper for TTL support
     System.setProperty("zookeeper.extendedTypesEnabled", "true");
     String className = TestHelper.getTestClassName();
     String methodName = TestHelper.getTestMethodName();
@@ -479,19 +485,23 @@ public void testSyncDoUpdateWithTTL() {
     ZNRecord record = new ZNRecord("msg_0");
     ZkBaseDataAccessor<ZNRecord> accessor = Mockito.spy(new ZkBaseDataAccessor<ZNRecord>(_gZkClient));
 
+    // Step 2: Attempt to update without TTL (should fail)
     AccessResult result = accessor.doUpdate(path, new ZNRecordUpdater(record), AccessOption.PERSISTENT_WITH_TTL);
     // Fails as ttl is not provided when AccessOption.PERSISTENT_WITH_TTL is used
     Assert.assertEquals(result._retCode, RetCode.ERROR);
 
+    // Step 3: Update with TTL
     result = accessor.doUpdate(path, new ZNRecordUpdater(record), AccessOption.PERSISTENT_WITH_TTL, ttl);
     Assert.assertEquals(result._retCode, RetCode.OK);
     ZNRecord getRecord = _gZkClient.readData(path);
     Assert.assertNotNull(getRecord);
     Assert.assertEquals(getRecord.getId(), "msg_0");
+    // Step 4: Verify if the znode created is configured with TTL
     Mockito.verify(accessor, Mockito.times(1)).doCreate(Mockito.anyString(), Mockito.any(ZNRecord.class), Mockito.eq(AccessOption.PERSISTENT_WITH_TTL), Mockito.eq(ttl));
 
     Mockito.reset(accessor);
 
+    // Step 5: Update the record with a simple field and verify
     record.setSimpleField("key0", "value0");
     result = accessor.doUpdate(path, new ZNRecordUpdater(record), AccessOption.PERSISTENT_WITH_TTL, ttl);
     Assert.assertEquals(result._retCode, RetCode.OK);
@@ -500,8 +510,10 @@ public void testSyncDoUpdateWithTTL() {
     Assert.assertEquals(getRecord.getSimpleFields().size(), 1);
     Assert.assertNotNull(getRecord.getSimpleField("key0"));
     Assert.assertEquals(getRecord.getSimpleField("key0"), "value0");
+    // Step 6: Verify if the znode created is configured with TTL
     Mockito.verify(accessor, Mockito.times(0)).doCreate(Mockito.anyString(), Mockito.any(ZNRecord.class), Mockito.eq(AccessOption.PERSISTENT_WITH_TTL), Mockito.eq(ttl));
 
+    // Step 7: Clear the extended types property
     System.clearProperty("zookeeper.extendedTypesEnabled");
     System.out.println("END " + testName + " at " + new Date(System.currentTimeMillis()));
   }

helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBucketDataAccessor.java

@@ -60,7 +60,7 @@ public class TestZkBucketDataAccessor extends ZkTestBase {
   private final ZNRecord record = new ZNRecord(NAME_KEY);
 
   private HelixZkClient _zkClient;
-  private ZkBucketDataAccessor _bucketDataAccessor;
+  private BucketDataAccessor _bucketDataAccessor;
   private BaseDataAccessor<byte[]> _zkBaseDataAccessor;
   private BucketDataAccessor _fastGCBucketDataAccessor;
 
@@ -226,17 +226,17 @@ public void testGCScheduler() throws IOException, InterruptedException {
   @Test
   public void testGetAccessOption() {
     long ttl = 1000L;  // Example of a positive TTL
-    int result = _bucketDataAccessor.getAccessOption(ttl);
+    int result = ZkBucketDataAccessor.getAccessOption(ttl);
     Assert.assertEquals(AccessOption.PERSISTENT_WITH_TTL, result,
         "Expected PERSISTENT_WITH_TTL for positive znodeTTLms");
 
     ttl = 0L;  // Example of a zero TTL
-    result = _bucketDataAccessor.getAccessOption(ttl);
+    result = ZkBucketDataAccessor.getAccessOption(ttl);
     Assert.assertEquals(AccessOption.PERSISTENT, result,
         "Expected PERSISTENT for zero znodeTTLms");
 
     ttl = -100L;  // Example of a negative TTL
-    result = _bucketDataAccessor.getAccessOption(ttl);
+    result = ZkBucketDataAccessor.getAccessOption(ttl);
     Assert.assertEquals(AccessOption.PERSISTENT, result,
         "Expected PERSISTENT for negative znodeTTLms");
   }