Author: mahadev
Date: Thu Apr 9 21:02:49 2009
New Revision: 763799
URL: http://svn.apache.org/viewvc?rev=763799&view=rev
Log:
ZOOKEEPER-355. make validatePath non public in Zookeeper client api. (phunt via
mahadev)
Modified:
hadoop/zookeeper/trunk/CHANGES.txt
hadoop/zookeeper/trunk/src/c/src/zookeeper.c
hadoop/zookeeper/trunk/src/c/tests/TestClient.cc
hadoop/zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java
hadoop/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
hadoop/zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ClientTest.java
Modified: hadoop/zookeeper/trunk/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hadoop/zookeeper/trunk/CHANGES.txt?rev=763799&r1=763798&r2=763799&view=diff
==============================================================================
--- hadoop/zookeeper/trunk/CHANGES.txt (original)
+++ hadoop/zookeeper/trunk/CHANGES.txt Thu Apr 9 21:02:49 2009
@@ -45,6 +45,9 @@
ZOOKEEPER-347. zkfuse uses non-standard String. (patrick hunt via mahadev)
+ ZOOKEEPER-355. make validatePath non public in Zookeeper client api. (phunt
+via mahadev)
+
IMPROVEMENTS:
ZOOKEEPER-308. improve the atomic broadcast performance 3x.
(breed via mahadev)
Modified: hadoop/zookeeper/trunk/src/c/src/zookeeper.c
URL:
http://svn.apache.org/viewvc/hadoop/zookeeper/trunk/src/c/src/zookeeper.c?rev=763799&r1=763798&r2=763799&view=diff
==============================================================================
--- hadoop/zookeeper/trunk/src/c/src/zookeeper.c (original)
+++ hadoop/zookeeper/trunk/src/c/src/zookeeper.c Thu Apr 9 21:02:49 2009
@@ -1902,7 +1902,7 @@
return rc;
}
-static int isValidPath(const char* path) {
+static int isValidPath(const char* path, const int flags) {
if (path == 0)
return 0;
const int len = strlen(path);
@@ -1912,7 +1912,7 @@
return 0;
if (len == 1) // done checking - it's the root
return 1;
- if (path[len - 1] == '/')
+ if (path[len - 1] == '/' && !(flags & ZOO_SEQUENCE))
return 0;
char lastc = '/';
@@ -1926,11 +1926,13 @@
} else if (c == '/' && lastc == '/') {
return 0;
} else if (c == '.' && lastc == '.') {
- if (path[i-2] == '/' && ((i + 1 == len) || path[i+1] == '/')) {
+ if (path[i-2] == '/' && (((i + 1 == len) && !(flags & ZOO_SEQUENCE))
+ || path[i+1] == '/')) {
return 0;
}
} else if (c == '.') {
- if ((path[i-1] == '/') && ((i + 1 == len) || path[i+1] == '/')) {
+ if ((path[i-1] == '/') && (((i + 1 == len) && !(flags & ZOO_SEQUENCE))
+ || path[i+1] == '/')) {
return 0;
}
} else if (c > 0x00 && c < 0x1f) {
@@ -1956,7 +1958,7 @@
struct GetDataRequest req = { (char*)path, watcher!=0 };
int rc;
- if (zh==0 || !isValidPath(path))
+ if (zh==0 || !isValidPath(path, 0))
return ZBADARGUMENTS;
if (is_unrecoverable(zh))
return ZINVALIDSTATE;
@@ -1988,7 +1990,7 @@
struct SetDataRequest req;
int rc;
- if (zh==0 || !isValidPath(path))
+ if (zh==0 || !isValidPath(path, 0))
return ZBADARGUMENTS;
if (is_unrecoverable(zh))
return ZINVALIDSTATE;
@@ -2015,7 +2017,7 @@
}
int zoo_acreate(zhandle_t *zh, const char *path, const char *value,
- int valuelen, const struct ACL_vector *acl_entries, int ephemeral,
+ int valuelen, const struct ACL_vector *acl_entries, int flags,
string_completion_t completion, const void *data)
{
struct oarchive *oa;
@@ -2023,13 +2025,13 @@
struct CreateRequest req;
int rc;
- if (zh==0 || !isValidPath(path))
+ if (zh==0 || !isValidPath(path, flags))
return ZBADARGUMENTS;
if (is_unrecoverable(zh))
return ZINVALIDSTATE;
oa = create_buffer_oarchive();
req.path = (char*)path;
- req.flags = ephemeral;
+ req.flags = flags;
req.data.buff = (char*)value;
req.data.len = valuelen;
if (acl_entries == 0) {
@@ -2063,7 +2065,7 @@
struct DeleteRequest req;
int rc;
- if (zh==0 || !isValidPath(path))
+ if (zh==0 || !isValidPath(path, 0))
return ZBADARGUMENTS;
if (is_unrecoverable(zh))
return ZINVALIDSTATE;
@@ -2102,7 +2104,7 @@
struct ExistsRequest req = {(char*)path, watcher!=0 };
int rc;
- if (zh==0 || !isValidPath(path))
+ if (zh==0 || !isValidPath(path, 0))
return ZBADARGUMENTS;
if (is_unrecoverable(zh))
return ZINVALIDSTATE;
@@ -2141,7 +2143,7 @@
struct GetChildrenRequest req={(char*)path, watcher!=0 };
int rc;
- if (zh==0 || !isValidPath(path))
+ if (zh==0 || !isValidPath(path, 0))
return ZBADARGUMENTS;
if (is_unrecoverable(zh))
return ZINVALIDSTATE;
@@ -2172,7 +2174,7 @@
struct SyncRequest req;
int rc;
- if (zh==0 || !isValidPath(path))
+ if (zh==0 || !isValidPath(path, 0))
return ZBADARGUMENTS;
if (is_unrecoverable(zh))
return ZINVALIDSTATE;
@@ -2204,7 +2206,7 @@
struct GetACLRequest req;
int rc;
- if (zh==0 || !isValidPath(path))
+ if (zh==0 || !isValidPath(path, 0))
return ZBADARGUMENTS;
if (is_unrecoverable(zh))
return ZINVALIDSTATE;
@@ -2235,7 +2237,7 @@
struct SetACLRequest req;
int rc;
- if (zh==0 || !isValidPath(path))
+ if (zh==0 || !isValidPath(path, 0))
return ZBADARGUMENTS;
if (is_unrecoverable(zh))
return ZINVALIDSTATE;
Modified: hadoop/zookeeper/trunk/src/c/tests/TestClient.cc
URL:
http://svn.apache.org/viewvc/hadoop/zookeeper/trunk/src/c/tests/TestClient.cc?rev=763799&r1=763798&r2=763799&view=diff
==============================================================================
--- hadoop/zookeeper/trunk/src/c/tests/TestClient.cc (original)
+++ hadoop/zookeeper/trunk/src/c/tests/TestClient.cc Thu Apr 9 21:02:49 2009
@@ -296,6 +296,16 @@
path, "", 0, &ZOO_OPEN_ACL_UNSAFE, 0, 0, 0));
}
+ static void verifyCreateFailsSeq(const char *path, zhandle_t *zk) {
+ CPPUNIT_ASSERT_EQUAL((int)ZBADARGUMENTS, zoo_create(zk,
+ path, "", 0, &ZOO_OPEN_ACL_UNSAFE, ZOO_SEQUENCE, 0, 0));
+ }
+
+ static void verifyCreateOkSeq(const char *path, zhandle_t *zk) {
+ CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_create(zk,
+ path, "", 0, &ZOO_OPEN_ACL_UNSAFE, ZOO_SEQUENCE, 0, 0));
+ }
+
/**
returns false if the vectors dont match
@@ -370,6 +380,16 @@
verifyCreateFails("foo", zk);
verifyCreateFails("a", zk);
+ // verify that trailing fails, except for seq which adds suffix
+ verifyCreateOk("/createseq", zk);
+ verifyCreateFails("/createseq/", zk);
+ verifyCreateOkSeq("/createseq/", zk);
+ verifyCreateOkSeq("/createseq/.", zk);
+ verifyCreateOkSeq("/createseq/..", zk);
+ verifyCreateFailsSeq("/createseq//", zk);
+ verifyCreateFailsSeq("/createseq/./", zk);
+ verifyCreateFailsSeq("/createseq/../", zk);
+
verifyCreateOk("/.foo", zk);
verifyCreateOk("/.f.", zk);
verifyCreateOk("/..f", zk);
Modified:
hadoop/zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java
URL:
http://svn.apache.org/viewvc/hadoop/zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java?rev=763799&r1=763798&r2=763799&view=diff
==============================================================================
--- hadoop/zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java
(original)
+++ hadoop/zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java
Thu Apr 9 21:02:49 2009
@@ -33,6 +33,7 @@
import org.apache.zookeeper.AsyncCallback.StatCallback;
import org.apache.zookeeper.AsyncCallback.StringCallback;
import org.apache.zookeeper.AsyncCallback.VoidCallback;
+import org.apache.zookeeper.common.PathUtils;
import org.apache.zookeeper.data.ACL;
import org.apache.zookeeper.data.Stat;
import org.apache.zookeeper.proto.CreateRequest;
@@ -127,7 +128,7 @@
* the public methods will not be exposed as part of the ZooKeeper client
* API.
*/
- private class ZKWatchManager implements ClientWatchManager {
+ private static class ZKWatchManager implements ClientWatchManager {
private final Map<String, Set<Watcher>> dataWatches =
new HashMap<String, Set<Watcher>>();
private final Map<String, Set<Watcher>> existWatches =
@@ -500,7 +501,8 @@
CreateMode createMode)
throws KeeperException, InterruptedException
{
- validatePath(path);
+ // also handle the case where server will append name suffix
+ PathUtils.validatePath(path, createMode.isSequential());
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.create);
@@ -522,72 +524,6 @@
}
/**
- * Validate the provided znode path string
- * @param path znode path string
- * @throws IllegalArgumentException if the path is invalid
- */
- public static void validatePath(String path) throws
IllegalArgumentException {
- if (path == null) {
- throw new IllegalArgumentException("Path cannot be null");
- }
- if (path.length() == 0) {
- throw new IllegalArgumentException("Path length must be > 0");
- }
- if (path.charAt(0) != '/') {
- throw new IllegalArgumentException(
- "Path must start with / character");
- }
- if (path.length() == 1) { // done checking - it's the root
- return;
- }
- if (path.charAt(path.length() - 1) == '/') {
- throw new IllegalArgumentException(
- "Path must not end with / character");
- }
-
- String reason = null;
- char lastc = '/';
- char chars[] = path.toCharArray();
- char c;
- for (int i = 1; i < chars.length; lastc = chars[i], i++) {
- c = chars[i];
-
- if (c == 0) {
- reason = "null character not allowed @" + i;
- break;
- } else if (c == '/' && lastc == '/') {
- reason = "empty node name specified @" + i;
- break;
- } else if (c == '.' && lastc == '.') {
- if (chars[i-2] == '/' &&
- ((i + 1 == chars.length)
- || chars[i+1] == '/')) {
- reason = "relative paths not allowed @" + i;
- break;
- }
- } else if (c == '.') {
- if (chars[i-1] == '/' &&
- ((i + 1 == chars.length)
- || chars[i+1] == '/')) {
- reason = "relative paths not allowed @" + i;
- break;
- }
- } else if (c > '\u0000' && c < '\u001f'
- || c > '\u007f' && c < '\u009F'
- || c > '\ud800' && c < '\uf8ff'
- || c > '\ufff0' && c < '\uffff') {
- reason = "invalid charater @" + i;
- break;
- }
- }
-
- if (reason != null) {
- throw new IllegalArgumentException(
- "Invalid path string \"" + path + "\" caused by " +
reason);
- }
- }
-
- /**
* The Asynchronous version of create. The request doesn't actually until
* the asynchronous callback is called.
*
@@ -597,7 +533,8 @@
public void create(String path, byte data[], List<ACL> acl,
CreateMode createMode, StringCallback cb, Object ctx)
{
- validatePath(path);
+ // also handle the case where server will append name suffix
+ PathUtils.validatePath(path, createMode.isSequential());
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.create);
@@ -639,7 +576,7 @@
*/
public void delete(String path, int version) throws
InterruptedException, KeeperException {
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.delete);
@@ -659,7 +596,7 @@
* @see #delete(String, int)
*/
public void delete(String path, int version, VoidCallback cb, Object ctx) {
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.delete);
@@ -689,7 +626,7 @@
public Stat exists(String path, Watcher watcher) throws KeeperException,
InterruptedException
{
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.exists);
@@ -745,7 +682,7 @@
public void exists(String path, Watcher watcher, StatCallback cb,
Object ctx)
{
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.exists);
@@ -792,7 +729,7 @@
*/
public byte[] getData(String path, Watcher watcher, Stat stat)
throws KeeperException, InterruptedException {
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.getData);
@@ -844,7 +781,7 @@
* @see #getData(String, Watcher, Stat)
*/
public void getData(String path, Watcher watcher, DataCallback cb, Object
ctx) {
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.getData);
@@ -900,7 +837,7 @@
*/
public Stat setData(String path, byte data[], int version)
throws KeeperException, InterruptedException {
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.setData);
@@ -924,7 +861,7 @@
*/
public void setData(String path, byte data[], int version, StatCallback cb,
Object ctx) {
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.setData);
@@ -955,7 +892,7 @@
*/
public List<ACL> getACL(String path, Stat stat)
throws KeeperException, InterruptedException {
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.getACL);
@@ -977,7 +914,7 @@
* @see #getACL(String, Stat)
*/
public void getACL(String path, Stat stat, ACLCallback cb, Object ctx) {
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.getACL);
@@ -1011,7 +948,7 @@
*/
public Stat setACL(String path, List<ACL> acl, int version)
throws KeeperException, InterruptedException {
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.setACL);
@@ -1038,7 +975,7 @@
*/
public void setACL(String path, List<ACL> acl, int version,
StatCallback cb, Object ctx) {
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.setACL);
@@ -1075,7 +1012,7 @@
*/
public List<String> getChildren(String path, Watcher watcher)
throws KeeperException, InterruptedException {
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.getChildren);
@@ -1127,7 +1064,7 @@
*/
public void getChildren(String path, Watcher watcher, ChildrenCallback cb,
Object ctx) {
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.getChildren);
@@ -1162,7 +1099,7 @@
* @throws IllegalArgumentException if an invalid path is specified
*/
public void sync(String path, VoidCallback cb, Object ctx){
- validatePath(path);
+ PathUtils.validatePath(path);
RequestHeader h = new RequestHeader();
h.setType(ZooDefs.OpCode.sync);
Modified:
hadoop/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
URL:
http://svn.apache.org/viewvc/hadoop/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java?rev=763799&r1=763798&r2=763799&view=diff
==============================================================================
---
hadoop/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
(original)
+++
hadoop/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
Thu Apr 9 21:02:49 2009
@@ -30,9 +30,9 @@
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.ZooDefs;
-import org.apache.zookeeper.ZooKeeper;
import org.apache.zookeeper.KeeperException.Code;
import org.apache.zookeeper.ZooDefs.OpCode;
+import org.apache.zookeeper.common.PathUtils;
import org.apache.zookeeper.data.ACL;
import org.apache.zookeeper.data.Id;
import org.apache.zookeeper.data.StatPersisted;
@@ -205,13 +205,6 @@
Long.toHexString(request.sessionId));
throw new KeeperException.BadArgumentsException();
}
- try {
- ZooKeeper.validatePath(path);
- } catch(IllegalArgumentException ie) {
- LOG.warn("Invalid path " + path + " with session " +
- Long.toHexString(request.sessionId));
- throw new KeeperException.BadArgumentsException();
- }
if (!fixupACL(request.authInfo, createRequest.getAcl())) {
throw new KeeperException.InvalidACLException();
}
@@ -221,11 +214,19 @@
checkACL(zks, parentRecord.acl, ZooDefs.Perms.CREATE,
request.authInfo);
int parentCVersion = parentRecord.stat.getCversion();
- CreateMode createMode =
CreateMode.fromFlag(createRequest.getFlags());
+ CreateMode createMode =
+ CreateMode.fromFlag(createRequest.getFlags());
if (createMode.isSequential()) {
path = path + String.format("%010d", parentCVersion);
}
try {
+ PathUtils.validatePath(path);
+ } catch(IllegalArgumentException ie) {
+ LOG.warn("Invalid path " + path + " with session " +
+ Long.toHexString(request.sessionId));
+ throw new KeeperException.BadArgumentsException();
+ }
+ try {
if (getRecordForPath(path) != null) {
throw new KeeperException.NodeExistsException();
}
Modified:
hadoop/zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ClientTest.java
URL:
http://svn.apache.org/viewvc/hadoop/zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ClientTest.java?rev=763799&r1=763798&r2=763799&view=diff
==============================================================================
---
hadoop/zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ClientTest.java
(original)
+++
hadoop/zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ClientTest.java
Thu Apr 9 21:02:49 2009
@@ -487,6 +487,49 @@
verifyCreateFails("foo", zk);
verifyCreateFails("a", zk);
+
+ zk.create("/createseqpar", null, Ids.OPEN_ACL_UNSAFE,
+ CreateMode.PERSISTENT);
+ // next two steps - related to sequential processing
+ // 1) verify that empty child name fails if not sequential
+ try {
+ zk.create("/createseqpar/", null, Ids.OPEN_ACL_UNSAFE,
+ CreateMode.PERSISTENT);
+ assertTrue(false);
+ } catch(IllegalArgumentException be) {
+ // catch this.
+ }
+
+ // 2) verify that empty child name success if sequential
+ zk.create("/createseqpar/", null, Ids.OPEN_ACL_UNSAFE,
+ CreateMode.PERSISTENT_SEQUENTIAL);
+ zk.create("/createseqpar/.", null, Ids.OPEN_ACL_UNSAFE,
+ CreateMode.PERSISTENT_SEQUENTIAL);
+ zk.create("/createseqpar/..", null, Ids.OPEN_ACL_UNSAFE,
+ CreateMode.PERSISTENT_SEQUENTIAL);
+ try {
+ zk.create("/createseqpar//", null, Ids.OPEN_ACL_UNSAFE,
+ CreateMode.PERSISTENT_SEQUENTIAL);
+ assertTrue(false);
+ } catch(IllegalArgumentException be) {
+ // catch this.
+ }
+ try {
+ zk.create("/createseqpar/./", null, Ids.OPEN_ACL_UNSAFE,
+ CreateMode.PERSISTENT_SEQUENTIAL);
+ assertTrue(false);
+ } catch(IllegalArgumentException be) {
+ // catch this.
+ }
+ try {
+ zk.create("/createseqpar/../", null, Ids.OPEN_ACL_UNSAFE,
+ CreateMode.PERSISTENT_SEQUENTIAL);
+ assertTrue(false);
+ } catch(IllegalArgumentException be) {
+ // catch this.
+ }
+
+
//check for the code path that throws at server
PrepRequestProcessor.failCreate = true;
try {
