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 <r...@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());

Reply via email to