Return-Path: X-Original-To: apmail-curator-commits-archive@minotaur.apache.org Delivered-To: apmail-curator-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 11340182DE for ; Wed, 1 Jul 2015 23:01:19 +0000 (UTC) Received: (qmail 53263 invoked by uid 500); 1 Jul 2015 23:01:18 -0000 Delivered-To: apmail-curator-commits-archive@curator.apache.org Received: (qmail 53161 invoked by uid 500); 1 Jul 2015 23:01:18 -0000 Mailing-List: contact commits-help@curator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@curator.apache.org Delivered-To: mailing list commits@curator.apache.org Received: (qmail 52708 invoked by uid 99); 1 Jul 2015 23:01:18 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 01 Jul 2015 23:01:18 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 7ED01E3631; Wed, 1 Jul 2015 23:01:18 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: randgalt@apache.org To: commits@curator.apache.org Date: Wed, 01 Jul 2015 23:01:31 -0000 Message-Id: <8a11851121d1418ea5cfb9f4e4e071a5@git.apache.org> In-Reply-To: <670a3ea55ea342d2ad4795731839be3f@git.apache.org> References: <670a3ea55ea342d2ad4795731839be3f@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [14/20] curator git commit: creatingParentContainersIfNeeded for checkExists() was broken. Fixed and added a test 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/master Commit: 7ad12754a9f1bd4ac9242886c245e3e2d2fa7dc4 Parents: d678de0 Author: randgalt Authored: Tue Jun 23 17:58:28 2015 -0500 Committer: randgalt 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 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 Stat returnStat = null; if ( backgrounding.inBackground() ) { - client.processBackgroundOperation(new OperationAndData(this, path, backgrounding.getCallback(), null, backgrounding.getContext()), null); + OperationAndData operationAndData = new OperationAndData(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 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() + { + @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 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 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); } } }