From 43096c29232afb9adc5e24c2c798c0ea96bb5ae2 Mon Sep 17 00:00:00 2001 From: "geyi.11" Date: Tue, 25 Aug 2026 21:19:24 +0800 Subject: [PATCH] Fix TreeCacheIterator throwing NoSuchElementExceptions Fixes #1302 --- .../recipes/cache/TreeCacheIterator.java | 32 ++++---- .../cache/TestTreeCacheIteratorAndSize.java | 78 +++++++++++++++++++ 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCacheIterator.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCacheIterator.java index c73476a73..15cb47bf5 100644 --- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCacheIterator.java +++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache/TreeCacheIterator.java @@ -68,19 +68,25 @@ public ChildData next() { private void setNext() { if (current.node.children != null) { - stack.push(current); - current = new Current(current.node.children.values().iterator()); - } else - while (true) { - if (current.iterator.hasNext()) { - current.node = current.iterator.next(); - break; - } else if (stack.size() > 0) { - current = stack.pop(); - } else { - current = null; // done - break; - } + Iterator childIterator = + current.node.children.values().iterator(); + if (childIterator.hasNext()) { + stack.push(current); + current = new Current(childIterator); + return; } + } + + while (true) { + if (current.iterator.hasNext()) { + current.node = current.iterator.next(); + break; + } else if (stack.size() > 0) { + current = stack.pop(); + } else { + current = null; // done + break; + } + } } } diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCacheIteratorAndSize.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCacheIteratorAndSize.java index d724fec1d..5f6ef914d 100644 --- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCacheIteratorAndSize.java +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCacheIteratorAndSize.java @@ -30,6 +30,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ThreadLocalRandom; +import java.util.stream.Collectors; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.CuratorFrameworkFactory; import org.apache.curator.retry.RetryOneTime; @@ -190,4 +191,81 @@ public void testWithDeletedNodes() throws Exception { } } } + + /** + * TreeCache retains a non-null empty children map after a node's last child is deleted. + * iterator() must still work. + */ + @Test + public void testIteratorAfterLastChildRemoved() throws Exception { + try (CuratorFramework client = + CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1))) { + client.start(); + + try (TreeCache treeCache = new TreeCache(client, "/foo")) { + treeCache.start(); + + client.create().forPath("/foo"); + client.create().forPath("/foo/a"); + client.create().forPath("/foo/a/a1"); + client.create().forPath("/foo/a/a2"); + client.create().forPath("/foo/b"); + timing.sleepABit(); + + client.delete().forPath("/foo/a/a2"); + client.delete().forPath("/foo/a/a1"); + timing.sleepABit(); + + assertEquals(collectPaths(treeCache.iterator()), Sets.newHashSet("/foo", "/foo/a", "/foo/b")); + assertEquals(treeCache.size(), 3); + + client.delete().forPath("/foo/a"); + client.delete().forPath("/foo/b"); + timing.sleepABit(); + + assertEquals(collectPaths(treeCache.iterator()), Sets.newHashSet("/foo")); + assertEquals(treeCache.size(), 1); + + client.create().forPath("/foo/c"); + timing.sleepABit(); + + assertEquals(collectPaths(treeCache.iterator()), Sets.newHashSet("/foo", "/foo/c")); + assertEquals(treeCache.size(), 2); + } + } + } + + @Test + public void testCuratorCacheBridgeStreamAfterLastChildRemoved() throws Exception { + System.setProperty("curator-cache-bridge-force-tree-cache", "true"); + try (CuratorFramework client = + CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1))) { + client.start(); + client.create().forPath("/foo"); + client.create().forPath("/foo/child"); + + try (CuratorCacheBridge cache = + CuratorCache.bridgeBuilder(client, "/foo").build()) { + cache.start(); + timing.sleepABit(); + + client.delete().forPath("/foo/child"); + timing.sleepABit(); + + assertEquals( + cache.stream().map(ChildData::getPath).collect(Collectors.toSet()), Sets.newHashSet("/foo")); + assertEquals(cache.size(), 1); + } + } finally { + System.clearProperty("curator-cache-bridge-force-tree-cache"); + } + } + + private static Set collectPaths(Iterator iterator) { + Set paths = new HashSet<>(); + while (iterator.hasNext()) { + paths.add(iterator.next().getPath()); + } + return paths; + } }