Repository: curator Updated Branches: refs/heads/CURATOR-264 [created] f8f05be2e
CURATOR-45 added findAndDeleteProtectedNodeInBackground to handle cases where a protected node can get lost. However, the code wasn't correctly handling namespaces Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/f8f05be2 Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/f8f05be2 Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/f8f05be2 Branch: refs/heads/CURATOR-264 Commit: f8f05be2e097c4c9be65e5110a376d461fd9cd9a Parents: c8cc3f4 Author: randgalt <randg...@apache.org> Authored: Tue Sep 22 14:59:41 2015 -0500 Committer: randgalt <randg...@apache.org> Committed: Tue Sep 22 14:59:41 2015 -0500 ---------------------------------------------------------------------- .../framework/imps/CreateBuilderImpl.java | 24 +++++------ .../framework/imps/TestFrameworkEdges.java | 42 +++++++++++++++++++- 2 files changed, 53 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/f8f05be2/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java ---------------------------------------------------------------------- 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 index 7a4a96f..b72b7b6 100644 --- 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 @@ -464,13 +464,13 @@ class CreateBuilderImpl implements CreateBuilder, BackgroundOperation<PathAndByt } else { - String path = protectedPathInForeground(adjustedPath, data); + String path = protectedPathInForeground(givenPath, adjustedPath, data); returnPath = client.unfixForNamespace(path); } return returnPath; } - private String protectedPathInForeground(String adjustedPath, byte[] data) throws Exception + private String protectedPathInForeground(String givenPath, String adjustedPath, byte[] data) throws Exception { try { @@ -486,7 +486,7 @@ class CreateBuilderImpl implements CreateBuilder, BackgroundOperation<PathAndByt * register the znode to be sure it is deleted later. */ String localProtectedId = protectedId; - findAndDeleteProtectedNodeInBackground(adjustedPath, localProtectedId, null); + findAndDeleteProtectedNodeInBackground(givenPath, localProtectedId, null); /* * The current UUID is scheduled to be deleted, it is not safe to use it again. * If this builder is used again later create a new UUID @@ -635,7 +635,7 @@ class CreateBuilderImpl implements CreateBuilder, BackgroundOperation<PathAndByt if ( doProtected ) { // all retries have failed, findProtectedNodeInForeground(..) included, schedule a clean up - findAndDeleteProtectedNodeInBackground(path, protectedId, null); + findAndDeleteProtectedNodeInBackground(givenPath, protectedId, null); // assign a new id if this builder is used again later protectedId = UUID.randomUUID().toString(); } @@ -795,25 +795,25 @@ class CreateBuilderImpl implements CreateBuilder, BackgroundOperation<PathAndByt /** * Attempt to delete a protected znode * - * @param path the path - * @param protectedId the protected id - * @param callback callback to use, <code>null</code> to create a new one + * @param unAdjustedPath the path - raw without namespace resolution + * @param protectedId the protected id + * @param callback callback to use, <code>null</code> to create a new one */ - private void findAndDeleteProtectedNodeInBackground(String path, String protectedId, FindProtectedNodeCB callback) + private void findAndDeleteProtectedNodeInBackground(String unAdjustedPath, String protectedId, FindProtectedNodeCB callback) { if ( client.getState() == CuratorFrameworkState.STARTED ) { if ( callback == null ) { - callback = new FindProtectedNodeCB(path, protectedId); + callback = new FindProtectedNodeCB(unAdjustedPath, protectedId); } try { - client.getChildren().inBackground(callback).forPath(ZKPaths.getPathAndNode(path).getPath()); + client.getChildren().inBackground(callback).forPath(ZKPaths.getPathAndNode(unAdjustedPath).getPath()); } catch ( Exception e ) { - findAndDeleteProtectedNodeInBackground(path, protectedId, callback); + findAndDeleteProtectedNodeInBackground(unAdjustedPath, protectedId, callback); } } } @@ -830,7 +830,7 @@ class CreateBuilderImpl implements CreateBuilder, BackgroundOperation<PathAndByt } @Override - public void processResult(CuratorFramework client, CuratorEvent event) throws Exception + public void processResult(CuratorFramework ignoreClient, CuratorEvent event) throws Exception { if ( event.getResultCode() == KeeperException.Code.OK.intValue() ) { http://git-wip-us.apache.org/repos/asf/curator/blob/f8f05be2/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java index cd3ae77..95c3792 100644 --- a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java +++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java @@ -29,10 +29,10 @@ import org.apache.curator.framework.api.CuratorEventType; import org.apache.curator.framework.api.CuratorListener; import org.apache.curator.framework.state.ConnectionState; import org.apache.curator.framework.state.ConnectionStateListener; +import org.apache.curator.retry.RetryNTimes; import org.apache.curator.retry.RetryOneTime; import org.apache.curator.test.BaseClassForTests; import org.apache.curator.test.KillSession; -import org.apache.curator.test.TestingServer; import org.apache.curator.test.Timing; import org.apache.curator.utils.CloseableUtils; import org.apache.curator.utils.ZKPaths; @@ -43,6 +43,7 @@ import org.apache.zookeeper.Watcher; import org.apache.zookeeper.data.Stat; import org.testng.Assert; import org.testng.annotations.Test; +import java.util.List; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CountDownLatch; @@ -56,6 +57,45 @@ public class TestFrameworkEdges extends BaseClassForTests private final Timing timing = new Timing(); @Test + public void testProtectedCreateNodeDeletion() throws Exception + { + CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), 1, new RetryNTimes(0, 0)); + try + { + client.start(); + + for ( int i = 0; i < 2; ++i ) + { + CuratorFramework localClient = (i == 0) ? client : client.usingNamespace("nm"); + localClient.create().forPath("/parent"); + Assert.assertEquals(localClient.getChildren().forPath("/parent").size(), 0); + + CreateBuilderImpl createBuilder = (CreateBuilderImpl)localClient.create(); + createBuilder.failNextCreateForTesting = true; + try + { + createBuilder.withProtection().forPath("/parent/test"); + Assert.fail("failNextCreateForTesting should have caused a ConnectionLossException"); + } + catch ( KeeperException.ConnectionLossException e ) + { + // ignore, correct + } + + timing.sleepABit(); + List<String> children = localClient.getChildren().forPath("/parent"); + Assert.assertEquals(children.size(), 0, children.toString()); // protected mode should have deleted the node + + localClient.delete().forPath("/parent"); + } + } + finally + { + CloseableUtils.closeQuietly(client); + } + } + + @Test public void testPathsFromProtectingInBackground() throws Exception { for ( CreateMode mode : CreateMode.values() )
