You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ra...@apache.org on 2017/05/08 03:42:57 UTC

curator git commit: some refactoring, refinement

Repository: curator
Updated Branches:
  refs/heads/CURATOR-397 c002e22e5 -> e4a7e0917


some refactoring, refinement


Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/e4a7e091
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/e4a7e091
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/e4a7e091

Branch: refs/heads/CURATOR-397
Commit: e4a7e09172c7d9d14081233dec88690ecba6df9e
Parents: c002e22
Author: randgalt <ra...@apache.org>
Authored: Mon May 8 05:42:17 2017 +0200
Committer: randgalt <ra...@apache.org>
Committed: Mon May 8 05:42:17 2017 +0200

----------------------------------------------------------------------
 .../apache/curator/x/async/modeled/ZPath.java   | 17 +----
 .../x/async/modeled/details/ModelSpecImpl.java  | 28 +++-----
 .../x/async/modeled/details/ZPathImpl.java      | 75 +++++++++-----------
 .../x/async/modeled/TestModeledFramework.java   | 27 ++++---
 .../curator/x/async/modeled/TestZPath.java      | 41 +++++++----
 5 files changed, 90 insertions(+), 98 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/e4a7e091/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 fae4e3a..21982a9 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
@@ -42,14 +42,9 @@ public interface ZPath extends Resolvable
     String idName = System.getProperty("curator-zpath-id-name", "{id}");
 
     /**
-     * Return the root path: "/"
-     *
-     * @return root path
+     * The root path: "/"
      */
-    static ZPath root()
-    {
-        return ZPathImpl.root;
-    }
+    ZPath root = ZPathImpl.root;
 
     /**
      * Take a string path and return a ZPath
@@ -242,14 +237,6 @@ public interface ZPath extends Resolvable
     String fullPath();
 
     /**
-     * The string parent path of this ZPath
-     *
-     * @return parent path
-     * @throws java.util.NoSuchElementException if this is the root ZPath
-     */
-    String parentPath();
-
-    /**
      * The node name at this ZPath
      *
      * @return name

http://git-wip-us.apache.org/repos/asf/curator/blob/e4a7e091/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ModelSpecImpl.java
----------------------------------------------------------------------
diff --git a/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ModelSpecImpl.java b/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ModelSpecImpl.java
index 24abd67..3a4a504 100644
--- a/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ModelSpecImpl.java
+++ b/curator-x-async/src/main/java/org/apache/curator/x/async/modeled/details/ModelSpecImpl.java
@@ -33,7 +33,6 @@ import org.apache.zookeeper.data.ACL;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicReference;
 
 public class ModelSpecImpl<T> implements ModelSpec<T>, SchemaValidator
 {
@@ -44,7 +43,7 @@ public class ModelSpecImpl<T> implements ModelSpec<T>, SchemaValidator
     private final Set<CreateOption> createOptions;
     private final Set<DeleteOption> deleteOptions;
     private final long ttl;
-    private final AtomicReference<Schema> schema = new AtomicReference<>();
+    private volatile Schema schema = null;
 
     public ModelSpecImpl(ZPath path, ModelSerializer<T> serializer, CreateMode createMode, List<ACL> aclList, Set<CreateOption> createOptions, Set<DeleteOption> deleteOptions, long ttl)
     {
@@ -126,13 +125,17 @@ public class ModelSpecImpl<T> implements ModelSpec<T>, SchemaValidator
     @Override
     public Schema schema()
     {
-        Schema schemaValue = schema.get();
-        if ( schemaValue == null )
+        if ( schema == null )
         {
-            schemaValue = makeSchema();
-            schema.compareAndSet(null, schemaValue);
+            schema = Schema.builder(path.toSchemaPathPattern())
+                .dataValidator(this)
+                .ephemeral(createMode.isEphemeral() ? Schema.Allowance.MUST : Schema.Allowance.CANNOT)
+                .canBeDeleted(true)
+                .sequential(createMode.isSequential() ? Schema.Allowance.MUST : Schema.Allowance.CANNOT)
+                .watched(Schema.Allowance.CAN)
+                .build();
         }
-        return schemaValue;
+        return schema;
     }
 
     @Override
@@ -219,15 +222,4 @@ public class ModelSpecImpl<T> implements ModelSpec<T>, SchemaValidator
         }
         return true;
     }
-
-    private Schema makeSchema()
-    {
-        return Schema.builder(path.toSchemaPathPattern())
-            .dataValidator(this)
-            .ephemeral(createMode.isEphemeral() ? Schema.Allowance.MUST : Schema.Allowance.CANNOT)
-            .canBeDeleted(true)
-            .sequential(createMode.isSequential() ? Schema.Allowance.MUST : Schema.Allowance.CANNOT)
-            .watched(Schema.Allowance.CAN)
-            .build();
-    }
 }

http://git-wip-us.apache.org/repos/asf/curator/blob/e4a7e091/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 ce47a60..4250969 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
@@ -42,8 +42,9 @@ public class ZPathImpl implements ZPath
 
     private final List<String> nodes;
     private final boolean isResolved;
-    private volatile String fullPathCache = null;
-    private volatile  String parentPathCache = null;
+    private volatile String fullPath = null;
+    private volatile ZPath parent = null;
+    private volatile Pattern schema = null;
 
     public static ZPath parse(String fullPath, UnaryOperator<String> nameFilter)
     {
@@ -116,7 +117,11 @@ public class ZPathImpl implements ZPath
     public ZPath parent()
     {
         checkRootAccess();
-        return new ZPathImpl(nodes.subList(0, nodes.size() - 1), null);
+        if ( parent == null )
+        {
+            parent = new ZPathImpl(nodes.subList(0, nodes.size() - 1), null);
+        }
+        return parent;
     }
 
     @Override
@@ -143,22 +148,22 @@ public class ZPathImpl implements ZPath
     @Override
     public Pattern toSchemaPathPattern()
     {
-        return Pattern.compile(fullPath() + PATH_SEPARATOR + ".*");
+        if ( schema == null )
+        {
+            schema = Pattern.compile(buildFullPath(s -> s.equals(parameterNodeName) ? ".*" : s));
+        }
+        return schema;
     }
 
     @Override
     public String fullPath()
     {
         checkResolved();
-        return buildFullPath(false);
-    }
-
-    @Override
-    public String parentPath()
-    {
-        checkRootAccess();
-        checkResolved();
-        return buildFullPath(true);
+        if ( fullPath == null )
+        {
+            fullPath = buildFullPath(s -> s);
+        }
+        return fullPath;
     }
 
     @Override
@@ -243,17 +248,24 @@ public class ZPathImpl implements ZPath
         Preconditions.checkState(isResolved, "This ZPath has not been resolved");
     }
 
-    private String buildFullPath(boolean parent)
+    private static void validate(String nodeName)
     {
-        String cachedValue = parent ? parentPathCache : fullPathCache;
-        if ( cachedValue != null )
+        if ( parameterNodeName.equals(Objects.requireNonNull(nodeName, "nodeName cannot be null")) )
+        {
+            return;
+        }
+        if ( nodeName.equals(PATH_SEPARATOR) )
         {
-            return cachedValue;
+            return;
         }
+        PathUtils.validatePath(PATH_SEPARATOR + nodeName);
+    }
 
+    private String buildFullPath(UnaryOperator<String> filter)
+    {
         boolean addSeparator = false;
         StringBuilder str = new StringBuilder();
-        int size = parent ? (nodes.size() - 1) : nodes.size();
+        int size = nodes.size();
         int parameterIndex = 0;
         for ( int i = 0; i < size; ++i )
         {
@@ -261,31 +273,8 @@ public class ZPathImpl implements ZPath
             {
                 str.append(PATH_SEPARATOR);
             }
-            str.append(nodes.get(i));
-        }
-
-        String value = str.toString();
-        if ( parent )
-        {
-            parentPathCache = value;
-        }
-        else
-        {
-            fullPathCache = value;
-        }
-        return value;
-    }
-
-    private static void validate(String nodeName)
-    {
-        if ( parameterNodeName.equals(Objects.requireNonNull(nodeName, "nodeName cannot be null")) )
-        {
-            return;
+            str.append(filter.apply(nodes.get(i)));
         }
-        if ( nodeName.equals(PATH_SEPARATOR) )
-        {
-            return;
-        }
-        PathUtils.validatePath(PATH_SEPARATOR + nodeName);
+        return str.toString();
     }
 }

http://git-wip-us.apache.org/repos/asf/curator/blob/e4a7e091/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestModeledFramework.java
----------------------------------------------------------------------
diff --git a/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestModeledFramework.java b/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestModeledFramework.java
index f14c485..a7884ee 100644
--- a/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestModeledFramework.java
+++ b/curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestModeledFramework.java
@@ -40,10 +40,9 @@ public class TestModeledFramework extends CompletableBaseClassForTests
 {
     private static final ZPath path = ZPath.parse("/test/path");
     private CuratorFramework rawClient;
-    private JacksonModelSerializer<TestModel> serializer;
-    private JacksonModelSerializer<TestNewerModel> newSerializer;
     private ModelSpec<TestModel> modelSpec;
     private ModelSpec<TestNewerModel> newModelSpec;
+    private AsyncCuratorFramework async;
 
     @BeforeMethod
     @Override
@@ -53,9 +52,10 @@ public class TestModeledFramework extends CompletableBaseClassForTests
 
         rawClient = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1));
         rawClient.start();
+        async = AsyncCuratorFramework.wrap(rawClient);
 
-        serializer = new JacksonModelSerializer<>(TestModel.class);
-        newSerializer = new JacksonModelSerializer<>(TestNewerModel.class);
+        JacksonModelSerializer<TestModel> serializer = JacksonModelSerializer.build(TestModel.class);
+        JacksonModelSerializer<TestNewerModel> newSerializer = JacksonModelSerializer.build(TestNewerModel.class);
 
         modelSpec = ModelSpec.builder(path, serializer).build();
         newModelSpec = ModelSpec.builder(path, newSerializer).build();
@@ -74,7 +74,7 @@ public class TestModeledFramework extends CompletableBaseClassForTests
     {
         TestModel rawModel = new TestModel("John", "Galt", "1 Galt's Gulch", 42, BigInteger.valueOf(1));
         TestModel rawModel2 = new TestModel("Wayne", "Rooney", "Old Trafford", 10, BigInteger.valueOf(1));
-        ModeledFramework<TestModel> client = ModeledFramework.wrap(AsyncCuratorFramework.wrap(rawClient), modelSpec);
+        ModeledFramework<TestModel> client = ModeledFramework.wrap(async, modelSpec);
         AsyncStage<String> stage = client.set(rawModel);
         Assert.assertNull(stage.event());
         complete(stage, (s, e) -> Assert.assertNotNull(s));
@@ -89,10 +89,10 @@ public class TestModeledFramework extends CompletableBaseClassForTests
     public void testBackwardCompatibility()
     {
         TestNewerModel rawNewModel = new TestNewerModel("John", "Galt", "1 Galt's Gulch", 42, BigInteger.valueOf(1), 100);
-        ModeledFramework<TestNewerModel> clientForNew = ModeledFramework.wrap(AsyncCuratorFramework.wrap(rawClient), newModelSpec);
+        ModeledFramework<TestNewerModel> clientForNew = ModeledFramework.wrap(async, newModelSpec);
         complete(clientForNew.set(rawNewModel), (s, e) -> Assert.assertNotNull(s));
 
-        ModeledFramework<TestModel> clientForOld = ModeledFramework.wrap(AsyncCuratorFramework.wrap(rawClient), modelSpec);
+        ModeledFramework<TestModel> clientForOld = ModeledFramework.wrap(async, modelSpec);
         complete(clientForOld.read(), (model, e) -> Assert.assertTrue(rawNewModel.equalsOld(model)));
     }
 
@@ -100,7 +100,7 @@ public class TestModeledFramework extends CompletableBaseClassForTests
     public void testWatched() throws InterruptedException
     {
         CountDownLatch latch = new CountDownLatch(1);
-        ModeledFramework<TestModel> client = ModeledFramework.builder(AsyncCuratorFramework.wrap(rawClient), modelSpec).watched().build();
+        ModeledFramework<TestModel> client = ModeledFramework.builder(async, modelSpec).watched().build();
         client.checkExists().event().whenComplete((event, ex) -> latch.countDown());
         timing.sleepABit();
         Assert.assertEquals(latch.getCount(), 1);
@@ -112,7 +112,7 @@ public class TestModeledFramework extends CompletableBaseClassForTests
     public void testGetChildren()
     {
         TestModel model = new TestModel("John", "Galt", "1 Galt's Gulch", 42, BigInteger.valueOf(1));
-        ModeledFramework<TestModel> client = ModeledFramework.builder(AsyncCuratorFramework.wrap(rawClient), modelSpec).build();
+        ModeledFramework<TestModel> client = ModeledFramework.builder(async, modelSpec).build();
         complete(client.at("one").set(model));
         complete(client.at("two").set(model));
         complete(client.at("three").set(model));
@@ -120,4 +120,13 @@ public class TestModeledFramework extends CompletableBaseClassForTests
         Set<ZPath> expected = Sets.newHashSet(path.at("one"), path.at("two"), path.at("three"));
         complete(client.children(), (children, e) -> Assert.assertEquals(Sets.newHashSet(children), expected));
     }
+
+    @Test
+    public void testBadNode()
+    {
+        complete(async.create().forPath(modelSpec.path().fullPath(), "fubar".getBytes()));
+
+        ModeledFramework<TestModel> client = ModeledFramework.builder(async, modelSpec).watched().build();
+        complete(client.read().whenComplete((model, e) -> Assert.assertTrue(e instanceof RuntimeException)));
+    }
 }

http://git-wip-us.apache.org/repos/asf/curator/blob/e4a7e091/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 1e98e9f..2f52238 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
@@ -30,33 +30,35 @@ public class TestZPath
     @Test
     public void testRoot()
     {
-        Assert.assertEquals(ZPath.root().nodeName(), ZKPaths.PATH_SEPARATOR);
-        Assert.assertEquals(ZPath.root(), ZPathImpl.root);
-        Assert.assertTrue(ZPath.root().isRoot());
-        Assert.assertEquals(ZPath.root().at("foo").parent(), ZPath.root());
-        Assert.assertTrue(ZPath.root().at("foo").parent().isRoot());
+        Assert.assertEquals(ZPath.root.nodeName(), ZKPaths.PATH_SEPARATOR);
+        Assert.assertEquals(ZPath.root, ZPathImpl.root);
+        Assert.assertTrue(ZPath.root.isRoot());
+        Assert.assertEquals(ZPath.root.at("foo").parent(), ZPath.root);
+        Assert.assertTrue(ZPath.root.at("foo").parent().isRoot());
     }
 
     @Test
     public void testBasic()
     {
-        ZPath path = ZPath.root().at("one").at("two");
+        ZPath path = ZPath.root.at("one").at("two");
         Assert.assertFalse(path.isRoot());
-        Assert.assertEquals(path, ZPath.root().at("one").at("two"));
-        Assert.assertNotEquals(path, ZPath.root().at("onex").at("two"));
+        Assert.assertEquals(path, ZPath.root.at("one").at("two"));
+        Assert.assertNotEquals(path, ZPath.root.at("onex").at("two"));
         Assert.assertEquals(path.nodeName(), "two");
         Assert.assertEquals(path.fullPath(), "/one/two");
-        Assert.assertEquals(path.parentPath(), "/one");
+        Assert.assertEquals(path.parent().fullPath(), "/one");
+        Assert.assertEquals(path.fullPath(), "/one/two");       // call twice to test the internal cache
+        Assert.assertEquals(path.parent().fullPath(), "/one");  // call twice to test the internal cache
 
-        Assert.assertTrue(path.startsWith(ZPath.root().at("one")));
-        Assert.assertFalse(path.startsWith(ZPath.root().at("two")));
+        Assert.assertTrue(path.startsWith(ZPath.root.at("one")));
+        Assert.assertFalse(path.startsWith(ZPath.root.at("two")));
     }
 
     @Test
     public void testParsing()
     {
-        Assert.assertEquals(ZPath.parse("/"), ZPath.root());
-        Assert.assertEquals(ZPath.parse("/one/two/three"), ZPath.root().at("one").at("two").at("three"));
+        Assert.assertEquals(ZPath.parse("/"), ZPath.root);
+        Assert.assertEquals(ZPath.parse("/one/two/three"), ZPath.root.at("one").at("two").at("three"));
         Assert.assertEquals(ZPath.parse("/one/two/three"), ZPath.from("one", "two", "three"));
         Assert.assertEquals(ZPath.parseWithIds("/one/{id}/two/{id}"), ZPath.from("one", parameterNodeName, "two", parameterNodeName));
     }
@@ -74,4 +76,17 @@ public class TestZPath
         ZPath path = ZPath.from("one", parameterNodeName, "two", parameterNodeName);
         Assert.assertEquals(path.resolved("a", "b"), ZPath.from("one", "a", "two", "b"));
     }
+
+    @Test
+    public void testSchema()
+    {
+        ZPath path = ZPath.from("one", parameterNodeName, "two", parameterNodeName);
+        Assert.assertEquals(path.toSchemaPathPattern().toString(), "/one/.*/two/.*");
+        path = ZPath.parse("/one/two/three");
+        Assert.assertEquals(path.toSchemaPathPattern().toString(), "/one/two/three");
+        path = ZPath.parseWithIds("/one/{id}/three");
+        Assert.assertEquals(path.toSchemaPathPattern().toString(), "/one/.*/three");
+        path = ZPath.parseWithIds("/{id}/{id}/three");
+        Assert.assertEquals(path.toSchemaPathPattern().toString(), "/.*/.*/three");
+    }
 }