You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ti...@apache.org on 2023/04/12 02:24:37 UTC
[curator] branch master updated: CURATOR-609. ModeledCache should not deserialize empty ZNodes on deletion (#396)
This is an automated email from the ASF dual-hosted git repository.
tison pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/curator.git
The following commit(s) were added to refs/heads/master by this push:
new e4ad57e3 CURATOR-609. ModeledCache should not deserialize empty ZNodes on deletion (#396)
e4ad57e3 is described below
commit e4ad57e3600ccb5473544b7577584e25c780944f
Author: Ryan Ruel <ry...@ryanruel.com>
AuthorDate: Tue Apr 11 22:24:31 2023 -0400
CURATOR-609. ModeledCache should not deserialize empty ZNodes on deletion (#396)
Co-authored-by: Ryan Ruel <rr...@akamai.com>
---
.../x/async/modeled/details/ModeledCacheImpl.java | 15 +++-
.../async/modeled/TestCachedModeledFramework.java | 93 ++++++++++++++++++++++
2 files changed, 105 insertions(+), 3 deletions(-)
diff --git a/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ModeledCacheImpl.java b/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ModeledCacheImpl.java
index b2f28fab..7bef5e37 100644
--- a/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ModeledCacheImpl.java
+++ b/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ModeledCacheImpl.java
@@ -189,9 +189,18 @@ class ModeledCacheImpl<T> implements TreeCacheListener, ModeledCache<T>
{
ZPath path = ZPath.parse(event.getData().getPath());
Entry<T> entry = entries.remove(path);
- T model = (entry != null) ? entry.model : serializer.deserialize(event.getData().getData());
- Stat stat = (entry != null) ? entry.stat : event.getData().getStat();
- accept(ModeledCacheListener.Type.NODE_REMOVED, path, stat, model);
+ T model = null;
+ if (entry != null) {
+ model = entry.model;
+ }
+ else if (event.getData().getData() != null) {
+ model = serializer.deserialize(event.getData().getData());
+ }
+ if (model != null) {
+ Stat stat = (entry != null) ? entry.stat : event.getData().getStat();
+ accept(ModeledCacheListener.Type.NODE_REMOVED, path, stat, model);
+ }
+
break;
}
diff --git a/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestCachedModeledFramework.java b/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestCachedModeledFramework.java
index e310f5d4..692417d0 100644
--- a/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestCachedModeledFramework.java
+++ b/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestCachedModeledFramework.java
@@ -24,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
+
import com.google.common.collect.Sets;
import org.apache.curator.framework.state.ConnectionState;
import org.apache.curator.test.Timing;
@@ -31,6 +32,7 @@ import org.apache.curator.test.compatibility.CuratorTestBase;
import org.apache.curator.x.async.modeled.cached.CachedModeledFramework;
import org.apache.curator.x.async.modeled.cached.ModeledCacheListener;
import org.apache.curator.x.async.modeled.models.TestModel;
+import org.apache.zookeeper.data.Stat;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
@@ -42,6 +44,8 @@ import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Semaphore;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@@ -181,6 +185,95 @@ public class TestCachedModeledFramework extends TestModeledFrameworkBase
}
}
+ // Verify the CachedModeledFramework does not attempt to deserialize empty ZNodes on deletion using the Jackson
+ // model serializer.
+ // See: CURATOR-609
+ @Test
+ public void testEmptyNodeJacksonDeserialization()
+ {
+ final TestModel model = new TestModel("a", "b", "c", 20, BigInteger.ONE);
+ verifyEmptyNodeDeserialization(model, modelSpec);
+ }
+
+ // Verify the CachedModeledFramework does not attempt to deserialize empty ZNodes on deletion using the raw
+ // model serializer.
+ // See: CURATOR-609
+ @Test
+ public void testEmptyNodeRawDeserialization()
+ {
+ final byte[] byteModel = {0x01, 0x02, 0x03};
+ final ModelSpec<byte[]> byteModelSpec = ModelSpec.builder(path, ModelSerializer.raw).build();
+ verifyEmptyNodeDeserialization(byteModel, byteModelSpec);
+ }
+
+ private <T> void verifyEmptyNodeDeserialization(T model, ModelSpec<T> parentModelSpec)
+ {
+ // The sub-path is the ZNode that will be removed that does not contain any model data. Their should be no
+ // attempt to deserialize this empty ZNode.
+ final String subPath = parentModelSpec.path().toString() + "/sub";
+
+ final String testModelPath = subPath + "/test";
+ final String signalModelPath = subPath + "/signal";
+
+ final CountDownLatch latch = new CountDownLatch(1);
+
+ final AtomicBoolean modelWasNull = new AtomicBoolean(false);
+ final AtomicReference<Exception> caughtHandleException = new AtomicReference<>(null);
+
+ // Create a custom listener to signal the end of the test and ensure that nothing is thrown.
+ final ModeledCacheListener<T> listener = new ModeledCacheListener<T>() {
+ @Override
+ public void accept(Type t, ZPath p, Stat s, T m)
+ {
+ // We don't expect the handler to be called with a null model.
+ if (m == null) {
+ modelWasNull.set(true);
+ }
+
+ if (t == ModeledCacheListener.Type.NODE_ADDED && p.toString().equals(signalModelPath)) {
+ latch.countDown();
+ }
+ }
+
+ public void handleException(Exception e)
+ {
+ caughtHandleException.set(e);
+ }
+ };
+
+ final ModelSerializer<T> serializer = parentModelSpec.serializer();
+
+ // Create a cache client to watch the parent path.
+ try (CachedModeledFramework<T> cacheClient = ModeledFramework.wrap(async, parentModelSpec).cached())
+ {
+ cacheClient.listenable().addListener(listener);
+
+ ModelSpec<T> testModelSpec = ModelSpec.builder(ZPath.parse(testModelPath), serializer).build();
+ ModeledFramework<T> testModelClient = ModeledFramework.wrap(async, testModelSpec);
+
+ ModelSpec<T> signalModelSpec = ModelSpec.builder(ZPath.parse(signalModelPath), serializer).build();
+ ModeledFramework<T> signalModelClient = ModeledFramework.wrap(async, signalModelSpec);
+
+ cacheClient.start();
+
+ // Create the test model (with sub-path), creating the initial parent path structure.
+ complete(testModelClient.set(model));
+
+ // Delete the test model, then delete the sub-path. As the sub-path ZNode is empty, we expect that this
+ // should not throw an exception.
+ complete(testModelClient.delete());
+ complete(testModelClient.unwrap().delete().forPath(subPath));
+
+ // Finally, create the signal model purely to signal the end of the test.
+ complete(signalModelClient.set(model));
+
+ assertTrue(timing.awaitLatch(latch));
+
+ assertFalse(modelWasNull.get(), "Listener should never be called with a null model");
+ assertNull(caughtHandleException.get(), "Exception should not have been handled by listener");
+ }
+ }
+
private <T, R> Set<R> toSet(Stream<T> stream, Function<? super T, ? extends R> mapper)
{
return stream.map(mapper).collect(Collectors.toSet());