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());