Repository: curator Updated Branches: refs/heads/CURATOR-479-2 [created] 273832442
CURATOR-479 CachedModeledFrameworkImpl.children() and CachedModeledFrameworkImpl.childrenAsZNodes() were not implemented correctly but neither was the previous commit for this. This is the correct fix. Previous commit was returning full tree, should be 1 level only Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/27383244 Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/27383244 Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/27383244 Branch: refs/heads/CURATOR-479-2 Commit: 273832442298b9d8779c1eb64d881ebd668fee8b Parents: 2a9f04a Author: randgalt <[email protected]> Authored: Mon Dec 10 15:40:46 2018 -0500 Committer: randgalt <[email protected]> Committed: Mon Dec 10 15:40:46 2018 -0500 ---------------------------------------------------------------------- .../apache/curator/x/async/modeled/ZPath.java | 16 +---- .../details/CachedModeledFrameworkImpl.java | 4 +- .../x/async/modeled/details/ZPathImpl.java | 22 ------- .../modeled/TestCachedModeledFramework.java | 65 +++++++++++++------- .../curator/x/async/modeled/TestZPath.java | 10 --- 5 files changed, 46 insertions(+), 71 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/27383244/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/ZPath.java ---------------------------------------------------------------------- diff --git a/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/ZPath.java b/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/ZPath.java index 3b0e47c..70ac536 100644 --- a/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/ZPath.java +++ b/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/ZPath.java @@ -249,7 +249,7 @@ public interface ZPath extends Resolvable /** * Return true if this path starts with the given path. i.e. - * <code>ZPath.parse("/one/two/three").startsWith(ZPath.parse("/one/two"))</code> returns true + * <code>ZPath.from("/one/two/three").startsWith(ZPath.from("/one/two"))</code> returns true * * @param path base path * @return true/false @@ -257,20 +257,6 @@ public interface ZPath extends Resolvable boolean startsWith(ZPath path); /** - * Return true if this path is a parent, grandparent, etc. of the given path i.e. - * <code>ZPath.parse("/one").isParentOf(ZPath.parse("/one/two/three"))</code> returns true. - * However, <code>ZPath.from("/one/two/three").isParentOf(ZPath.from("/one/two/three"))</code> - * returns false. - * - * @param path base path - * @return true/false - */ - default boolean isParentOf(ZPath path) - { - return path.startsWith(this) && !path.fullPath().equals(fullPath()); - } - - /** * The string full path that this ZPath represents * * @return full path http://git-wip-us.apache.org/repos/asf/curator/blob/27383244/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/CachedModeledFrameworkImpl.java ---------------------------------------------------------------------- diff --git a/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/CachedModeledFrameworkImpl.java b/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/CachedModeledFrameworkImpl.java index 726b476..c897b4e 100644 --- a/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/CachedModeledFrameworkImpl.java +++ b/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/CachedModeledFrameworkImpl.java @@ -248,7 +248,7 @@ class CachedModeledFrameworkImpl<T> implements CachedModeledFramework<T> List<ZPath> paths = cache.currentChildren(client.modelSpec().path()) .keySet() .stream() - .filter(path -> client.modelSpec().path().isParentOf(path)) + .filter(path -> !path.isRoot() && path.parent().equals(client.modelSpec().path())) .collect(Collectors.toList()); return completed(paths); } @@ -259,7 +259,7 @@ class CachedModeledFrameworkImpl<T> implements CachedModeledFramework<T> List<ZNode<T>> nodes = cache.currentChildren(client.modelSpec().path()) .entrySet() .stream() - .filter(e -> client.modelSpec().path().isParentOf(e.getKey())) + .filter(e -> !e.getKey().isRoot() && e.getKey().parent().equals(client.modelSpec().path())) .map(Map.Entry::getValue) .collect(Collectors.toList()); return completed(nodes); http://git-wip-us.apache.org/repos/asf/curator/blob/27383244/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ZPathImpl.java ---------------------------------------------------------------------- diff --git a/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ZPathImpl.java b/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ZPathImpl.java index 65b25d5..fff742e 100644 --- a/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ZPathImpl.java +++ b/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ZPathImpl.java @@ -18,7 +18,6 @@ */ package org.apache.curator.x.async.modeled.details; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import org.apache.curator.x.async.modeled.NodeName; @@ -145,27 +144,6 @@ public class ZPathImpl implements ZPath return (nodes.size() >= rhs.nodes.size()) && nodes.subList(0, rhs.nodes.size()).equals(rhs.nodes); } - @VisibleForTesting - public boolean defaultIsParentOf(ZPath path) - { - return ZPath.super.isParentOf(path); - } - - @Override - public boolean isParentOf(ZPath path) - { - ZPathImpl rhs; - if ( path instanceof ZPathImpl ) - { - rhs = (ZPathImpl)path; - } - else - { - rhs = parseInternal(path.fullPath(), s -> s); - } - return (rhs.nodes.size() > nodes.size()) && rhs.nodes.subList(0, nodes.size()).equals(nodes); - } - @Override public Pattern toSchemaPathPattern() { http://git-wip-us.apache.org/repos/asf/curator/blob/27383244/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestCachedModeledFramework.java ---------------------------------------------------------------------- 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 73dc017..825d4b7 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 @@ -18,6 +18,7 @@ */ package org.apache.curator.x.async.modeled; +import com.google.common.collect.Sets; import org.apache.curator.framework.state.ConnectionState; import org.apache.curator.test.Timing; import org.apache.curator.x.async.modeled.cached.CachedModeledFramework; @@ -27,10 +28,14 @@ import org.testng.Assert; import org.testng.annotations.Test; import java.io.IOException; import java.math.BigInteger; -import java.util.HashSet; +import java.util.Arrays; +import java.util.List; import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Semaphore; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; public class TestCachedModeledFramework extends TestModeledFrameworkBase { @@ -97,46 +102,62 @@ public class TestCachedModeledFramework extends TestModeledFrameworkBase } @Test - public void testChildren() throws Exception + public void testChildren() { TestModel parent = new TestModel("a", "b", "c", 20, BigInteger.ONE); TestModel child1 = new TestModel("d", "e", "f", 1, BigInteger.ONE); TestModel child2 = new TestModel("g", "h", "i", 1, BigInteger.ONE); + TestModel grandChild1 = new TestModel("j", "k", "l", 10, BigInteger.ONE); + TestModel grandChild2 = new TestModel("m", "n", "0", 5, BigInteger.ONE); + try (CachedModeledFramework<TestModel> client = ModeledFramework.wrap(async, modelSpec).cached()) { - Semaphore semaphore = new Semaphore(0); - client.listenable().addListener((t, p, s, m) -> - { - if (m.equals(child2)) - { - semaphore.release(); - } - }); + CountDownLatch latch = new CountDownLatch(5); + client.listenable().addListener((t, p, s, m) -> latch.countDown()); client.start(); complete(client.child("p").set(parent)); complete(client.child("p").child("c1").set(child1)); complete(client.child("p").child("c2").set(child2)); - Assert.assertTrue(timing.forWaiting().acquireSemaphore(semaphore)); + complete(client.child("p").child("c1").child("g1").set(grandChild1)); + complete(client.child("p").child("c2").child("g2").set(grandChild2)); + Assert.assertTrue(timing.awaitLatch(latch)); complete(client.child("p").children(), (v, e) -> { - Assert.assertEquals(v.size(), 2); - Assert.assertTrue(v.contains(client.child("p").child("c1").modelSpec().path())); - Assert.assertTrue(v.contains(client.child("p").child("c2").modelSpec().path())); + List<ZPath> paths = Arrays.asList( + client.child("p").child("c1").modelSpec().path(), + client.child("p").child("c2").modelSpec().path() + ); + Assert.assertEquals(v, paths); }); complete(client.child("p").childrenAsZNodes(), (v, e) -> { - Assert.assertEquals(v.size(), 2); - Set<TestModel> children = new HashSet<>(); - children.add(child1); - children.add(child2); - for (ZNode<TestModel> node: v) - { - Assert.assertTrue(children.contains(node.model())); - } + Set<TestModel> cachedModels = toSet(v.stream(), ZNode::model); + Assert.assertEquals(cachedModels, Sets.newHashSet(child1, child2)); + + // verify that the same nodes are returned from the uncached method + complete(ModeledFramework.wrap(async, modelSpec).child("p").childrenAsZNodes(), (v2, e2) -> { + Set<TestModel> uncachedModels = toSet(v2.stream(), ZNode::model); + Assert.assertEquals(cachedModels, uncachedModels); + }); + }); + + complete(client.child("p").child("c1").childrenAsZNodes(), (v, e) -> + { + Assert.assertEquals(toSet(v.stream(), ZNode::model), Sets.newHashSet(grandChild1)); + }); + + complete(client.child("p").child("c2").childrenAsZNodes(), (v, e) -> + { + Assert.assertEquals(toSet(v.stream(), ZNode::model), Sets.newHashSet(grandChild2)); }); } } + + private <T, R> Set<R> toSet(Stream<T> stream, Function<? super T, ? extends R> mapper) + { + return stream.map(mapper).collect(Collectors.toSet()); + } } http://git-wip-us.apache.org/repos/asf/curator/blob/27383244/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestZPath.java ---------------------------------------------------------------------- diff --git a/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestZPath.java b/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestZPath.java index 894d0cd..d2c24da 100644 --- a/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestZPath.java +++ b/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestZPath.java @@ -59,16 +59,6 @@ public class TestZPath Assert.assertTrue(checkIdLike.isResolved()); checkIdLike = ZPath.parse("/one/" + ZPath.parameter("others") + "/three"); Assert.assertTrue(checkIdLike.isResolved()); - - Assert.assertTrue(ZPath.parse("/one").isParentOf(ZPath.parse("/one/two/three"))); - Assert.assertTrue(path.isParentOf(path.child("one"))); - Assert.assertFalse(path.isParentOf(path)); - Assert.assertFalse(path.isParentOf(path.parent())); - - // check default implementation - Assert.assertTrue(((ZPathImpl)path).defaultIsParentOf(path.child("one"))); - Assert.assertFalse(((ZPathImpl)path).defaultIsParentOf(path)); - Assert.assertFalse(((ZPathImpl)path).defaultIsParentOf(path.parent())); } @Test
