curator-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From randg...@apache.org
Subject [3/3] git commit: CURATOR-58
Date Fri, 08 Nov 2013 16:40:50 GMT
CURATOR-58

EnsurePath and creatingParentsIfNeeded() were not using the ACLProvider to create parent nodes.
The fix for this
is complicated by the fact that ACLProvider is in the framework module but ZKPaths is in the
client module. Workaround
by introducing a InternalACLProvider and making ACLProvider a "synonym" for it. It can now
be passed to ZKPaths.mkdirs().


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

Branch: refs/heads/master
Commit: 16b9438fe34dc00cdd527e53dc7e71b9cf13b695
Parents: c5a7ee4
Author: randgalt <randgalt@apache.org>
Authored: Fri Nov 8 08:38:57 2013 -0800
Committer: randgalt <randgalt@apache.org>
Committed: Fri Nov 8 08:38:57 2013 -0800

----------------------------------------------------------------------
 .../org/apache/curator/utils/EnsurePath.java    | 77 ++++++++-------
 .../curator/utils/InternalACLProvider.java      | 42 +++++++++
 .../java/org/apache/curator/utils/ZKPaths.java  | 36 ++++++-
 .../curator/framework/api/ACLProvider.java      |  3 +-
 .../framework/imps/CreateBuilderImpl.java       |  4 +-
 .../curator/framework/imps/NamespaceImpl.java   |  2 +-
 .../framework/recipes/locks/TestLockACLs.java   | 98 ++++++++++++++++++++
 7 files changed, 224 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/16b9438f/curator-client/src/main/java/org/apache/curator/utils/EnsurePath.java
----------------------------------------------------------------------
diff --git a/curator-client/src/main/java/org/apache/curator/utils/EnsurePath.java b/curator-client/src/main/java/org/apache/curator/utils/EnsurePath.java
index 2a6c0e9..15687c4 100644
--- a/curator-client/src/main/java/org/apache/curator/utils/EnsurePath.java
+++ b/curator-client/src/main/java/org/apache/curator/utils/EnsurePath.java
@@ -16,6 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+
 package org.apache.curator.utils;
 
 import org.apache.curator.CuratorZookeeperClient;
@@ -26,18 +27,18 @@ import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * <p>
- *     Utility to ensure that a particular path is created.
+ * Utility to ensure that a particular path is created.
  * </p>
- *
+ * <p/>
  * <p>
- *     The first time it is used, a synchronized call to {@link ZKPaths#mkdirs(ZooKeeper,
String)} is made to
- *     ensure that the entire path has been created (with an empty byte array if needed).
Subsequent
- *     calls with the instance are un-synchronized NOPs.
+ * The first time it is used, a synchronized call to {@link ZKPaths#mkdirs(ZooKeeper, String)}
is made to
+ * ensure that the entire path has been created (with an empty byte array if needed). Subsequent
+ * calls with the instance are un-synchronized NOPs.
  * </p>
- *
+ * <p/>
  * <p>
- *     Usage:<br/>
- *     <code><pre>
+ * Usage:<br/>
+ * <code><pre>
  *         EnsurePath       ensurePath = new EnsurePath(aFullPathToEnsure);
  *         ...
  *         String           nodePath = aFullPathToEnsure + "/foo";
@@ -51,11 +52,12 @@ import java.util.concurrent.atomic.AtomicReference;
  */
 public class EnsurePath
 {
-    private final String                    path;
-    private final boolean                   makeLastNode;
-    private final AtomicReference<Helper>   helper;
+    private final String path;
+    private final boolean makeLastNode;
+    private final InternalACLProvider aclProvider;
+    private final AtomicReference<Helper> helper;
 
-    private static final Helper             doNothingHelper = new Helper()
+    private static final Helper doNothingHelper = new Helper()
     {
         @Override
         public void ensure(CuratorZookeeperClient client, String path, final boolean makeLastNode)
throws Exception
@@ -66,7 +68,7 @@ public class EnsurePath
 
     private interface Helper
     {
-        public void     ensure(CuratorZookeeperClient client, String path, final boolean
makeLastNode) throws Exception;
+        public void ensure(CuratorZookeeperClient client, String path, final boolean makeLastNode)
throws Exception;
     }
 
     /**
@@ -74,7 +76,16 @@ public class EnsurePath
      */
     public EnsurePath(String path)
     {
-        this(path, null, true);
+        this(path, null, true, null);
+    }
+
+    /**
+     * @param path the full path to ensure
+     * @param aclProvider if not null, the ACL provider to use when creating parent nodes
+     */
+    public EnsurePath(String path, InternalACLProvider aclProvider)
+    {
+        this(path, null, true, aclProvider);
     }
 
     /**
@@ -84,9 +95,9 @@ public class EnsurePath
      * @param client ZK client
      * @throws Exception ZK errors
      */
-    public void     ensure(CuratorZookeeperClient client) throws Exception
+    public void ensure(CuratorZookeeperClient client) throws Exception
     {
-        Helper  localHelper = helper.get();
+        Helper localHelper = helper.get();
         localHelper.ensure(client, path, makeLastNode);
     }
 
@@ -98,13 +109,14 @@ public class EnsurePath
      */
     public EnsurePath excludingLast()
     {
-        return new EnsurePath(path, helper, false);
+        return new EnsurePath(path, helper, false, aclProvider);
     }
 
-    private EnsurePath(String path, AtomicReference<Helper> helper, boolean makeLastNode)
+    private EnsurePath(String path, AtomicReference<Helper> helper, boolean makeLastNode,
InternalACLProvider aclProvider)
     {
         this.path = path;
         this.makeLastNode = makeLastNode;
+        this.aclProvider = aclProvider;
         this.helper = (helper != null) ? helper : new AtomicReference<Helper>(new InitialHelper());
     }
 
@@ -113,13 +125,14 @@ public class EnsurePath
      *
      * @return the path being ensured
      */
-    public String getPath() {
+    public String getPath()
+    {
         return this.path;
     }
 
     private class InitialHelper implements Helper
     {
-        private boolean         isSet = false;  // guarded by synchronization
+        private boolean isSet = false;  // guarded by synchronization
 
         @Override
         public synchronized void ensure(final CuratorZookeeperClient client, final String
path, final boolean makeLastNode) throws Exception
@@ -127,20 +140,20 @@ public class EnsurePath
             if ( !isSet )
             {
                 RetryLoop.callWithRetry
-                (
-                    client,
-                    new Callable<Object>()
-                    {
-                        @Override
-                        public Object call() throws Exception
+                    (
+                        client,
+                        new Callable<Object>()
                         {
-                            ZKPaths.mkdirs(client.getZooKeeper(), path, makeLastNode);
-                            helper.set(doNothingHelper);
-                            isSet = true;
-                            return null;
+                            @Override
+                            public Object call() throws Exception
+                            {
+                                ZKPaths.mkdirs(client.getZooKeeper(), path, makeLastNode,
aclProvider);
+                                helper.set(doNothingHelper);
+                                isSet = true;
+                                return null;
+                            }
                         }
-                    }
-                );
+                    );
             }
         }
     }

http://git-wip-us.apache.org/repos/asf/curator/blob/16b9438f/curator-client/src/main/java/org/apache/curator/utils/InternalACLProvider.java
----------------------------------------------------------------------
diff --git a/curator-client/src/main/java/org/apache/curator/utils/InternalACLProvider.java
b/curator-client/src/main/java/org/apache/curator/utils/InternalACLProvider.java
new file mode 100644
index 0000000..661a55f
--- /dev/null
+++ b/curator-client/src/main/java/org/apache/curator/utils/InternalACLProvider.java
@@ -0,0 +1,42 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.curator.utils;
+
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.data.ACL;
+import java.util.List;
+
+public interface InternalACLProvider
+{
+    /**
+     * Return the ACL list to use by default (usually {@link ZooDefs.Ids#OPEN_ACL_UNSAFE}).
+     *
+     * @return default ACL list
+     */
+    public List<ACL> getDefaultAcl();
+
+    /**
+     * Return the ACL list to use for the given path
+     *
+     * @param path path (NOTE: might be null)
+     * @return ACL list
+     */
+    public List<ACL>        getAclForPath(String path);
+}

http://git-wip-us.apache.org/repos/asf/curator/blob/16b9438f/curator-client/src/main/java/org/apache/curator/utils/ZKPaths.java
----------------------------------------------------------------------
diff --git a/curator-client/src/main/java/org/apache/curator/utils/ZKPaths.java b/curator-client/src/main/java/org/apache/curator/utils/ZKPaths.java
index 4f178d5..04b2141 100644
--- a/curator-client/src/main/java/org/apache/curator/utils/ZKPaths.java
+++ b/curator-client/src/main/java/org/apache/curator/utils/ZKPaths.java
@@ -23,6 +23,7 @@ import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.ACL;
 import java.util.Collections;
 import java.util.List;
 
@@ -123,7 +124,7 @@ public class ZKPaths
     public static void mkdirs(ZooKeeper zookeeper, String path)
         throws InterruptedException, KeeperException
     {
-        mkdirs(zookeeper, path, true);
+        mkdirs(zookeeper, path, true, null);
     }
 
     /**
@@ -140,6 +141,24 @@ public class ZKPaths
     public static void mkdirs(ZooKeeper zookeeper, String path, boolean makeLastNode)
         throws InterruptedException, KeeperException
     {
+        mkdirs(zookeeper, path, makeLastNode, null);
+    }
+
+    /**
+     * Make sure all the nodes in the path are created. NOTE: Unlike File.mkdirs(), Zookeeper
doesn't distinguish
+     * between directories and files. So, every node in the path is created. The data for
each node is an empty blob
+     *
+     * @param zookeeper the client
+     * @param path      path to ensure
+     * @param makeLastNode if true, all nodes are created. If false, only the parent nodes
are created
+     * @throws InterruptedException thread interruption
+     * @throws org.apache.zookeeper.KeeperException
+     *                              Zookeeper errors
+     * @param aclProvider if not null, the ACL provider to use when creating parent nodes
+     */
+    public static void mkdirs(ZooKeeper zookeeper, String path, boolean makeLastNode, InternalACLProvider
aclProvider)
+        throws InterruptedException, KeeperException
+    {
         PathUtils.validatePath(path);
 
         int pos = 1; // skip first slash, root is guaranteed to exist
@@ -164,7 +183,20 @@ public class ZKPaths
             {
                 try
                 {
-                    zookeeper.create(subPath, new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+                    List<ACL> acl = null;
+                    if ( aclProvider != null )
+                    {
+                        acl = aclProvider.getAclForPath(path);
+                        if ( acl == null )
+                        {
+                            acl = aclProvider.getDefaultAcl();
+                        }
+                    }
+                    if ( acl == null )
+                    {
+                        acl = ZooDefs.Ids.OPEN_ACL_UNSAFE;
+                    }
+                    zookeeper.create(subPath, new byte[0], acl, CreateMode.PERSISTENT);
                 }
                 catch ( KeeperException.NodeExistsException e )
                 {

http://git-wip-us.apache.org/repos/asf/curator/blob/16b9438f/curator-framework/src/main/java/org/apache/curator/framework/api/ACLProvider.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/api/ACLProvider.java
b/curator-framework/src/main/java/org/apache/curator/framework/api/ACLProvider.java
index 4563e00..e0c5ba9 100644
--- a/curator-framework/src/main/java/org/apache/curator/framework/api/ACLProvider.java
+++ b/curator-framework/src/main/java/org/apache/curator/framework/api/ACLProvider.java
@@ -18,11 +18,12 @@
  */
 package org.apache.curator.framework.api;
 
+import org.apache.curator.utils.InternalACLProvider;
 import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.data.ACL;
 import java.util.List;
 
-public interface ACLProvider
+public interface ACLProvider extends InternalACLProvider
 {
     /**
      * Return the ACL list to use by default (usually {@link ZooDefs.Ids#OPEN_ACL_UNSAFE}).

http://git-wip-us.apache.org/repos/asf/curator/blob/16b9438f/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
b/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
index ebd342f..fa95b2c 100644
--- a/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
+++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
@@ -504,7 +504,7 @@ class CreateBuilderImpl implements CreateBuilder, BackgroundOperation<PathAndByt
             {
                 try
                 {
-                    ZKPaths.mkdirs(client.getZooKeeper(), mainOperationAndData.getData().getPath(),
false);
+                    ZKPaths.mkdirs(client.getZooKeeper(), mainOperationAndData.getData().getPath(),
false, client.getAclProvider());
                 }
                 catch ( KeeperException e )
                 {
@@ -679,7 +679,7 @@ class CreateBuilderImpl implements CreateBuilder, BackgroundOperation<PathAndByt
                             {
                                 if ( createParentsIfNeeded )
                                 {
-                                    ZKPaths.mkdirs(client.getZooKeeper(), path, false);
+                                    ZKPaths.mkdirs(client.getZooKeeper(), path, false, client.getAclProvider());
                                     createdPath = client.getZooKeeper().create(path, data,
acling.getAclList(path), createMode);
                                 }
                                 else

http://git-wip-us.apache.org/repos/asf/curator/blob/16b9438f/curator-framework/src/main/java/org/apache/curator/framework/imps/NamespaceImpl.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/NamespaceImpl.java
b/curator-framework/src/main/java/org/apache/curator/framework/imps/NamespaceImpl.java
index 169ded5..3fc8e80 100644
--- a/curator-framework/src/main/java/org/apache/curator/framework/imps/NamespaceImpl.java
+++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/NamespaceImpl.java
@@ -84,6 +84,6 @@ class NamespaceImpl
 
     EnsurePath newNamespaceAwareEnsurePath(String path)
     {
-        return new EnsurePath(fixForNamespace(path));
+        return new EnsurePath(fixForNamespace(path), client.getAclProvider());
     }
 }

http://git-wip-us.apache.org/repos/asf/curator/blob/16b9438f/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestLockACLs.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestLockACLs.java
b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestLockACLs.java
new file mode 100644
index 0000000..008b399
--- /dev/null
+++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestLockACLs.java
@@ -0,0 +1,98 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.curator.framework.recipes.locks;
+
+import com.google.common.io.Closeables;
+import org.apache.curator.RetryPolicy;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.apache.curator.framework.api.ACLProvider;
+import org.apache.curator.framework.recipes.BaseClassForTests;
+import org.apache.curator.retry.ExponentialBackoffRetry;
+import org.apache.curator.test.TestingServer;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+import java.util.Collections;
+import java.util.List;
+
+public class TestLockACLs extends BaseClassForTests
+{
+    private static final List<ACL> ACLS = Collections.singletonList(new ACL(ZooDefs.Perms.ALL,
new Id("ip", "127.0.0.1")));
+
+    private CuratorFramework createClient() throws Exception
+    {
+        RetryPolicy retryPolicy = new ExponentialBackoffRetry(1000, 3);
+        CuratorFramework client = CuratorFrameworkFactory.builder()
+            .namespace("ns")
+            .connectString(server.getConnectString())
+            .retryPolicy(retryPolicy)
+            .aclProvider(new MyACLProvider())
+            .build();
+        client.start();
+        return client;
+    }
+
+    @Test
+    public void testLockACLs() throws Exception
+    {
+        CuratorFramework client = createClient();
+        try
+        {
+            // Create a path directly and verify that MyACLProvider is being used
+            client.create().forPath("/foo");
+            Assert.assertNotNull(client.checkExists().forPath("/foo"));
+            Assert.assertEquals(ZooDefs.Perms.ALL, client.getACL().forPath("/foo").get(0).getPerms());
+            Assert.assertEquals("ip", client.getACL().forPath("/foo").get(0).getId().getScheme());
+            Assert.assertEquals("127.0.0.1", client.getACL().forPath("/foo").get(0).getId().getId());
+
+            // Now try creating a lock and we'll see that it incorrectly has the default
world ACLs
+            InterProcessReadWriteLock lock = new InterProcessReadWriteLock(client, "/bar");
+            InterProcessMutex writeLock = lock.writeLock();
+            writeLock.acquire();
+            Assert.assertNotNull(client.checkExists().forPath("/bar"));
+            Assert.assertEquals(ZooDefs.Perms.ALL, client.getACL().forPath("/bar").get(0).getPerms());
+            Assert.assertEquals("ip", client.getACL().forPath("/bar").get(0).getId().getScheme());
+            Assert.assertEquals("127.0.0.1", client.getACL().forPath("/bar").get(0).getId().getId());
+        }
+        finally
+        {
+            Closeables.closeQuietly(client);
+        }
+    }
+
+    public class MyACLProvider implements ACLProvider
+    {
+
+        @Override
+        public List<ACL> getDefaultAcl()
+        {
+            return ACLS;
+        }
+
+        @Override
+        public List<ACL> getAclForPath(String path)
+        {
+            return ACLS;
+        }
+    }
+}
\ No newline at end of file


Mime
View raw message