From b229a6fd33a79e5280f014dacedb2f1743fb4619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lakos=20P=C3=A9ter?= Date: Mon, 2 Jan 2017 17:41:56 +0100 Subject: [PATCH 1/2] CURATOR-373 Add option to PersistentNode to not overwrite/delete existing node --- .../recipes/nodes/PersistentNode.java | 38 +++-- .../recipes/nodes/TestPersistentNode.java | 23 ++++ ...sistentNodeWithOverwriteExistingFalse.java | 130 ++++++++++++++++++ 3 files changed, 181 insertions(+), 10 deletions(-) create mode 100644 curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentNodeWithOverwriteExistingFalse.java diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentNode.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentNode.java index 5f94ea197b..3a12942a6f 100644 --- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentNode.java +++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentNode.java @@ -69,6 +69,7 @@ public class PersistentNode implements Closeable private final AtomicBoolean authFailure = new AtomicBoolean(false); private final BackgroundCallback backgroundCallback; private final boolean useProtection; + private final AtomicBoolean overwriteExisting; private final CuratorWatcher watcher = new CuratorWatcher() { @Override @@ -139,16 +140,26 @@ private enum State CLOSED } + /** + * @see #PersistentNode(CuratorFramework, CreateMode, boolean, boolean, String, byte[]) + */ + public PersistentNode(CuratorFramework client, final CreateMode mode, boolean useProtection, final String basePath, byte[] initData) + { + this(client, mode, useProtection, true, basePath, initData); + } + /** * @param client client instance * @param mode creation mode * @param useProtection if true, call {@link CreateBuilder#withProtection()} * @param basePath the base path for the node * @param initData data for the node + * @param overwriteExisting if true, an already existing node will be overwritten with our data and it will be deleted on close */ - public PersistentNode(CuratorFramework client, final CreateMode mode, boolean useProtection, final String basePath, byte[] initData) + public PersistentNode(CuratorFramework client, final CreateMode mode, boolean useProtection, final boolean overwriteExisting, final String basePath, byte[] initData) { this.useProtection = useProtection; + this.overwriteExisting = new AtomicBoolean(overwriteExisting); this.client = Preconditions.checkNotNull(client, "client cannot be null"); this.basePath = PathUtils.validatePath(basePath); this.mode = Preconditions.checkNotNull(mode, "mode cannot be null"); @@ -186,7 +197,7 @@ else if ( event.getResultCode() == KeeperException.Code.OK.intValue() ) path = event.getName(); } - if ( path != null ) + if ( path != null && overwriteExisting.get() ) { try { @@ -226,7 +237,10 @@ else if ( event.getResultCode() == KeeperException.Code.NOAUTH.intValue() ) if ( nodeExists ) { - client.setData().inBackground(setDataCallback).forPath(getActualPath(), getData()); + if ( overwriteExisting.get() ) + { + client.setData().inBackground(setDataCallback).forPath(getActualPath(), getData()); + } } else { @@ -246,6 +260,7 @@ private void initialisationComplete() { localLatch.countDown(); } + overwriteExisting.set(true); } /** @@ -287,14 +302,17 @@ public void close() throws IOException client.getConnectionStateListenable().removeListener(connectionStateListener); - try + if ( overwriteExisting.get() ) { - deleteNode(); - } - catch ( Exception e ) - { - ThreadUtils.checkInterrupted(e); - throw new IOException(e); + try + { + deleteNode(); + } + catch (Exception e) + { + ThreadUtils.checkInterrupted(e); + throw new IOException(e); + } } } diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentNode.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentNode.java index ac8c65d304..987f920dcc 100644 --- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentNode.java +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentNode.java @@ -136,4 +136,27 @@ public void testQuickCloseNodeExists() throws Exception CloseableUtils.closeQuietly(client); } } + + @Test + public void testOverwriteExisting() throws Exception + { + Timing timing = new Timing(); + PersistentNode pen = null; + CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1)); + client.start(); + try + { + client.create().creatingParentsIfNeeded().forPath("/test/one/two", "abc123".getBytes()); + pen = new PersistentNode(client, CreateMode.PERSISTENT, false, "/test/one/two", "def456".getBytes()); + pen.start(); + timing.sleepABit(); + Assert.assertEquals(client.getData().forPath("/test/one/two"), "def456".getBytes()); + } + finally + { + CloseableUtils.closeQuietly(pen); + CloseableUtils.closeQuietly(client); + } + } + } diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentNodeWithOverwriteExistingFalse.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentNodeWithOverwriteExistingFalse.java new file mode 100644 index 0000000000..cbd4d1996f --- /dev/null +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentNodeWithOverwriteExistingFalse.java @@ -0,0 +1,130 @@ +package org.apache.curator.framework.recipes.nodes; + +import org.apache.curator.framework.CuratorFramework; +import org.apache.curator.framework.CuratorFrameworkFactory; +import org.apache.curator.retry.RetryOneTime; +import org.apache.curator.test.BaseClassForTests; +import org.apache.curator.test.Timing; +import org.apache.curator.utils.CloseableUtils; +import org.apache.zookeeper.CreateMode; +import org.testng.Assert; +import org.testng.annotations.Test; + +import java.util.concurrent.TimeUnit; + +public class TestPersistentNodeWithOverwriteExistingFalse extends BaseClassForTests +{ + @Test + public void testQuickSetData() throws Exception + { + final byte[] TEST_DATA = "hey".getBytes(); + final byte[] ALT_TEST_DATA = "there".getBytes(); + + Timing timing = new Timing(); + PersistentNode pen = null; + CuratorFramework client = CuratorFrameworkFactory + .newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1)); + try + { + client.start(); + pen = new PersistentNode(client, CreateMode.PERSISTENT, false, false, "/test", TEST_DATA); + pen.start(); + try + { + pen.setData(ALT_TEST_DATA); + Assert.fail("IllegalStateException should have been thrown"); + } + catch ( IllegalStateException dummy ) + { + // expected + } + } + finally + { + CloseableUtils.closeQuietly(pen); + CloseableUtils.closeQuietly(client); + } + } + + @Test + public void testBasic() throws Exception + { + final byte[] TEST_DATA = "hey".getBytes(); + + Timing timing = new Timing(); + PersistentNode pen = null; + CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1)); + try + { + client.start(); + pen = new PersistentNode(client, CreateMode.PERSISTENT, false, false, "/test", TEST_DATA); + pen.start(); + Assert.assertTrue(pen.waitForInitialCreate(timing.milliseconds(), TimeUnit.MILLISECONDS)); + client.close(); // cause session to end - force checks that node is persistent + + client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1)); + client.start(); + + byte[] bytes = client.getData().forPath("/test"); + Assert.assertEquals(bytes, TEST_DATA); + } + finally + { + CloseableUtils.closeQuietly(pen); + CloseableUtils.closeQuietly(client); + } + } + + @Test + public void testClose() throws Exception + { + Timing timing = new Timing(); + PersistentNode pen = null; + CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1)); + try + { + client.start(); + pen = new PersistentNode(client, CreateMode.PERSISTENT, false, false, "/test/one/two", new byte[0]); + pen.start(); + // we are only allowed to delete the node on close, once we know it belongs to us => wait for initial create + Assert.assertTrue(pen.waitForInitialCreate(timing.milliseconds(), TimeUnit.MILLISECONDS)); + pen.close(); + timing.sleepABit(); + Assert.assertNull(client.checkExists().forPath("/test/one/two")); + } + finally + { + CloseableUtils.closeQuietly(pen); + CloseableUtils.closeQuietly(client); + } + } + + @Test + public void testDontOverwriteOrDeleteExisting() throws Exception + { + final byte[] TEST_DATA = "hey".getBytes(); + final byte[] ALT_TEST_DATA = "there".getBytes(); + + Timing timing = new Timing(); + PersistentNode pen = null; + CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1)); + try + { + client.start(); + client.create().creatingParentsIfNeeded().forPath("/test/one/two", TEST_DATA); + + pen = new PersistentNode(client, CreateMode.PERSISTENT, false, false, "/test/one/two", ALT_TEST_DATA); + pen.start(); + timing.sleepABit(); + Assert.assertEquals(client.getData().forPath("/test/one/two"), TEST_DATA); + pen.close(); + timing.sleepABit(); + Assert.assertEquals(client.getData().forPath("/test/one/two"), TEST_DATA); + } + finally + { + CloseableUtils.closeQuietly(pen); + CloseableUtils.closeQuietly(client); + } + } +} From 069234a5355589db802b3fe9c57860bf5fa3f838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lakos=20P=C3=A9ter?= Date: Mon, 2 Jan 2017 17:53:05 +0100 Subject: [PATCH 2/2] CURATOR-373 add license to TestPersistentNodeWithOverwriteExistingFalse.java --- ...rsistentNodeWithOverwriteExistingFalse.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentNodeWithOverwriteExistingFalse.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentNodeWithOverwriteExistingFalse.java index cbd4d1996f..4d35cde3dc 100644 --- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentNodeWithOverwriteExistingFalse.java +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentNodeWithOverwriteExistingFalse.java @@ -1,3 +1,21 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ package org.apache.curator.framework.recipes.nodes; import org.apache.curator.framework.CuratorFramework;