curator-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From randg...@apache.org
Subject curator git commit: creatingParentContainersIfNeeded for checkExists() was broken. Fixed and added a test
Date Tue, 23 Jun 2015 22:58:37 GMT
Repository: curator
Updated Branches:
  refs/heads/CURATOR-222 d678de0bf -> 7ad12754a


creatingParentContainersIfNeeded for checkExists() was broken. Fixed and added a test


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

Branch: refs/heads/CURATOR-222
Commit: 7ad12754a9f1bd4ac9242886c245e3e2d2fa7dc4
Parents: d678de0
Author: randgalt <randgalt@apache.org>
Authored: Tue Jun 23 17:58:28 2015 -0500
Committer: randgalt <randgalt@apache.org>
Committed: Tue Jun 23 17:58:28 2015 -0500

----------------------------------------------------------------------
 .../framework/imps/ExistsBuilderImpl.java       | 85 +++++++++--------
 .../curator/framework/imps/TestFramework.java   | 97 +++++++++++++++-----
 2 files changed, 123 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/7ad12754/curator-framework/src/main/java/org/apache/curator/framework/imps/ExistsBuilderImpl.java
----------------------------------------------------------------------
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
index db7df9e..d4a059d 100644
--- 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
@@ -131,15 +131,8 @@ class ExistsBuilderImpl implements ExistsBuilder, BackgroundOperation<String>
             public void processResult(int rc, String path, Object ctx, Stat stat)
             {
                 trace.commit();
-                if ( (rc == KeeperException.Code.NONODE.intValue()) && createParentContainersIfNeeded
)
-                {
-                    CreateBuilderImpl.backgroundCreateParentsThenNode(client, operationAndData,
operationAndData.getData(), backgrounding, true);
-                }
-                else
-                {
-                    CuratorEvent event = new CuratorEventImpl(client, CuratorEventType.EXISTS,
rc, path, null, ctx, stat, null, null, null, null);
-                    client.processBackgroundOperation(operationAndData, event);
-                }
+                CuratorEvent event = new CuratorEventImpl(client, CuratorEventType.EXISTS,
rc, path, null, ctx, stat, null, null, null, null);
+                client.processBackgroundOperation(operationAndData, event);
             }
         };
         if ( watching.isWatched() )
@@ -160,7 +153,15 @@ class ExistsBuilderImpl implements ExistsBuilder, BackgroundOperation<String>
         Stat        returnStat = null;
         if ( backgrounding.inBackground() )
         {
-            client.processBackgroundOperation(new OperationAndData<String>(this, path,
backgrounding.getCallback(), null, backgrounding.getContext()), null);
+            OperationAndData<String> operationAndData = new OperationAndData<String>(this,
path, backgrounding.getCallback(), null, backgrounding.getContext());
+            if ( createParentContainersIfNeeded )
+            {
+                CreateBuilderImpl.backgroundCreateParentsThenNode(client, operationAndData,
operationAndData.getData(), backgrounding, true);
+            }
+            else
+            {
+                client.processBackgroundOperation(operationAndData, null);
+            }
         }
         else
         {
@@ -172,6 +173,40 @@ class ExistsBuilderImpl implements ExistsBuilder, BackgroundOperation<String>
 
     private Stat pathInForeground(final String path) throws Exception
     {
+        if ( createParentContainersIfNeeded )
+        {
+            final String parent = ZKPaths.getPathAndNode(path).getPath();
+            if ( !parent.equals(ZKPaths.PATH_SEPARATOR) )
+            {
+                TimeTrace   trace = client.getZookeeperClient().startTracer("ExistsBuilderImpl-Foreground-CreateParents");
+                RetryLoop.callWithRetry
+                (
+                    client.getZookeeperClient(),
+                    new Callable<Void>()
+                    {
+                        @Override
+                        public Void call() throws Exception
+                        {
+                            try
+                            {
+                                ZKPaths.mkdirs(client.getZooKeeper(), parent, true, client.getAclProvider(),
true);
+                            }
+                            catch ( KeeperException e )
+                            {
+                                // ignore
+                            }
+                            return null;
+                        }
+                    }
+                );
+                trace.commit();
+            }
+        }
+        return pathInForegroundStandard(path);
+    }
+
+    private Stat pathInForegroundStandard(final String path) throws Exception
+    {
         TimeTrace   trace = client.getZookeeperClient().startTracer("ExistsBuilderImpl-Foreground");
         Stat        returnStat = RetryLoop.callWithRetry
         (
@@ -182,21 +217,13 @@ class ExistsBuilderImpl implements ExistsBuilder, BackgroundOperation<String>
                 public Stat call() throws Exception
                 {
                     Stat    returnStat;
-                    try
+                    if ( watching.isWatched() )
                     {
-                        returnStat = callExists(path);
+                        returnStat = client.getZooKeeper().exists(path, true);
                     }
-                    catch ( KeeperException.NoNodeException e )
+                    else
                     {
-                        if ( createParentContainersIfNeeded )
-                        {
-                            ZKPaths.mkdirs(client.getZooKeeper(), path, false, client.getAclProvider(),
true);
-                            returnStat = callExists(path);
-                        }
-                        else
-                        {
-                            throw e;
-                        }
+                        returnStat = client.getZooKeeper().exists(path, watching.getWatcher());
                     }
                     return returnStat;
                 }
@@ -205,18 +232,4 @@ class ExistsBuilderImpl implements ExistsBuilder, BackgroundOperation<String>
         trace.commit();
         return returnStat;
     }
-
-    private Stat callExists(String path) throws Exception
-    {
-        Stat    returnStat;
-        if ( watching.isWatched() )
-        {
-            returnStat = client.getZooKeeper().exists(path, true);
-        }
-        else
-        {
-            returnStat = client.getZooKeeper().exists(path, watching.getWatcher());
-        }
-        return returnStat;
-    }
 }

http://git-wip-us.apache.org/repos/asf/curator/blob/7ad12754/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFramework.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFramework.java
b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFramework.java
index 0a7d8dc..528b4a5 100644
--- a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFramework.java
+++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFramework.java
@@ -134,7 +134,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -182,7 +182,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -239,7 +239,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -332,7 +332,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -415,7 +415,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -448,7 +448,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -478,7 +478,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -493,6 +493,57 @@ public class TestFramework extends BaseClassForTests
     }
 
     @Test
+    public void testExistsCreatingParents() throws Exception
+    {
+        CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(),
new RetryOneTime(1));
+        try
+        {
+            client.start();
+
+            Assert.assertNull(client.checkExists().forPath("/one/two"));
+            client.checkExists().creatingParentContainersIfNeeded().forPath("/one/two/three");
+            Assert.assertNotNull(client.checkExists().forPath("/one/two"));
+            Assert.assertNull(client.checkExists().forPath("/one/two/three"));
+            Assert.assertNull(client.checkExists().creatingParentContainersIfNeeded().forPath("/one/two/three"));
+        }
+        finally
+        {
+            CloseableUtils.closeQuietly(client);
+        }
+    }
+
+    @Test
+    public void testExistsCreatingParentsInBackground() throws Exception
+    {
+        CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(),
new RetryOneTime(1));
+        try
+        {
+            client.start();
+
+            Assert.assertNull(client.checkExists().forPath("/one/two"));
+
+            final CountDownLatch latch = new CountDownLatch(1);
+            BackgroundCallback callback = new BackgroundCallback()
+            {
+                @Override
+                public void processResult(CuratorFramework client, CuratorEvent event) throws
Exception
+                {
+                    latch.countDown();
+                }
+            };
+            client.checkExists().creatingParentContainersIfNeeded().inBackground(callback).forPath("/one/two/three");
+            Assert.assertTrue(new Timing().awaitLatch(latch));
+            Assert.assertNotNull(client.checkExists().forPath("/one/two"));
+            Assert.assertNull(client.checkExists().forPath("/one/two/three"));
+            Assert.assertNull(client.checkExists().creatingParentContainersIfNeeded().forPath("/one/two/three"));
+        }
+        finally
+        {
+            CloseableUtils.closeQuietly(client);
+        }
+    }
+
+    @Test
     public void testEnsurePathWithNamespace() throws Exception
     {
         final String namespace = "jz";
@@ -512,7 +563,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -544,7 +595,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -575,7 +626,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -611,7 +662,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -642,7 +693,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -679,7 +730,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -716,7 +767,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -734,7 +785,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -754,7 +805,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -774,7 +825,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -797,7 +848,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -850,7 +901,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -883,7 +934,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -937,7 +988,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -953,7 +1004,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 
@@ -971,7 +1022,7 @@ public class TestFramework extends BaseClassForTests
         }
         finally
         {
-            client.close();
+            CloseableUtils.closeQuietly(client);
         }
     }
 }


Mime
View raw message