You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@distributedlog.apache.org by sijie <gi...@git.apache.org> on 2017/04/27 06:34:51 UTC
[GitHub] incubator-distributedlog pull request #130: DL-199: Be able to support files...
GitHub user sijie opened a pull request:
https://github.com/apache/incubator-distributedlog/pull/130
DL-199: Be able to support filesystem-path like name
In order to support hierarchical namespace, we need to be able to support filesystem path like log name.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sijie/incubator-distributedlog DL_199
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-distributedlog/pull/130.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #130
----
commit 2837f09ef9d6f8b572f688fc0053cb4c45fa0840
Author: Sijie Guo <si...@apache.org>
Date: 2017-04-27T06:33:30Z
DL-199: Be able to support filesystem-path like name
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-distributedlog pull request #130: DL-199: Be able to support files...
Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on a diff in the pull request:
https://github.com/apache/incubator-distributedlog/pull/130#discussion_r118790221
--- Diff: distributedlog-core/src/main/java/org/apache/distributedlog/util/DLUtils.java ---
@@ -281,41 +281,89 @@ public static boolean isReservedStreamName(String name) {
}
/**
- * Validate the stream name.
+ * Validate the log name.
*
- * @param nameOfStream
- * name of stream
+ * @param logName
+ * name of log
* @throws InvalidStreamNameException
*/
- public static void validateName(String nameOfStream)
+ public static String validateAndNormalizeName(String logName)
throws InvalidStreamNameException {
- String reason = null;
- char chars[] = nameOfStream.toCharArray();
- char c;
- // validate the stream to see if meet zookeeper path's requirement
- for (int i = 0; i < chars.length; i++) {
- c = chars[i];
-
- if (c == 0) {
- reason = "null character not allowed @" + i;
- break;
- } else if (c == '/') {
- reason = "'/' 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 (isReservedStreamName(logName)) {
+ throw new InvalidStreamNameException(logName, "Log Name is reserved");
}
- if (null != reason) {
- throw new InvalidStreamNameException(nameOfStream, reason);
+
+ if (logName.charAt(0) == 47) {
+ validatePathName(logName);
+ return logName.substring(1);
+ } else {
+ validatePathName("/" + logName);
+ return logName;
}
- if (isReservedStreamName(nameOfStream)) {
- throw new InvalidStreamNameException(nameOfStream,
- "Stream Name is reserved");
+ }
+
+ private static void validatePathName(String logName) throws InvalidStreamNameException {
+ if (logName == null) {
+ throw new InvalidStreamNameException("Log name cannot be null");
+ } else if (logName.length() == 0) {
+ throw new InvalidStreamNameException("Log name length must be > 0");
+ } else if (logName.charAt(0) != 47) {
+ throw new InvalidStreamNameException("Log name must start with / character");
+ } else if (logName.length() != 1) {
+ if (logName.charAt(logName.length() - 1) == 47) {
+ throw new InvalidStreamNameException("Log name must not end with / character");
+ } else {
+ String reason = null;
+ char lastc = 47;
+ char[] chars = logName.toCharArray();
+
+ for (int i = 1; i < chars.length; ++i) {
+ char c = chars[i];
+ if (c == 0) {
+ reason = "null character not allowed @" + i;
+ break;
+ }
+
+ if (c == '<' || c == '>') {
+ reason = "< or > specified @" + i;
+ break;
+ }
+
+ if (c == ' ') {
--- End diff --
I believe it is covered in the last "else if" branch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-distributedlog pull request #130: DL-199: Be able to support files...
Posted by jiazhai <gi...@git.apache.org>.
Github user jiazhai commented on a diff in the pull request:
https://github.com/apache/incubator-distributedlog/pull/130#discussion_r117600405
--- Diff: distributedlog-core/src/test/java/org/apache/distributedlog/TestBKDistributedLogNamespace.java ---
@@ -143,7 +172,7 @@ public void testInvalidStreamName() throws Exception {
}
try {
- namespace.openLog("/test2");
+ namespace.openLog("/ test2");
fail("should fail to create invalid stream /test2");
--- End diff --
seems also need a space here: / test2
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-distributedlog pull request #130: DL-199: Be able to support files...
Posted by leighst <gi...@git.apache.org>.
Github user leighst commented on a diff in the pull request:
https://github.com/apache/incubator-distributedlog/pull/130#discussion_r121047018
--- Diff: distributedlog-core/src/main/java/org/apache/distributedlog/util/Utils.java ---
@@ -635,4 +636,16 @@ public static String getParent(final String path) {
return path.substring(0, lastIndex);
}
+ /**
+ * Validate the provided {@code logName}.
+ *
+ * @param logName name of the log
+ * @throws InvalidStreamNameException if the name of the log is not valid.
+ */
+ public static void validateLogName(String logName) throws InvalidStreamNameException {
--- End diff --
whats going on here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-distributedlog pull request #130: DL-199: Be able to support files...
Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on a diff in the pull request:
https://github.com/apache/incubator-distributedlog/pull/130#discussion_r118790138
--- Diff: distributedlog-core/src/main/java/org/apache/distributedlog/util/DLUtils.java ---
@@ -281,41 +281,89 @@ public static boolean isReservedStreamName(String name) {
}
/**
- * Validate the stream name.
+ * Validate the log name.
*
- * @param nameOfStream
- * name of stream
+ * @param logName
+ * name of log
* @throws InvalidStreamNameException
*/
- public static void validateName(String nameOfStream)
+ public static String validateAndNormalizeName(String logName)
throws InvalidStreamNameException {
- String reason = null;
- char chars[] = nameOfStream.toCharArray();
- char c;
- // validate the stream to see if meet zookeeper path's requirement
- for (int i = 0; i < chars.length; i++) {
- c = chars[i];
-
- if (c == 0) {
- reason = "null character not allowed @" + i;
- break;
- } else if (c == '/') {
- reason = "'/' 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 (isReservedStreamName(logName)) {
+ throw new InvalidStreamNameException(logName, "Log Name is reserved");
}
- if (null != reason) {
- throw new InvalidStreamNameException(nameOfStream, reason);
+
+ if (logName.charAt(0) == 47) {
--- End diff --
changed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-distributedlog pull request #130: DL-199: Be able to support files...
Posted by jiazhai <gi...@git.apache.org>.
Github user jiazhai commented on a diff in the pull request:
https://github.com/apache/incubator-distributedlog/pull/130#discussion_r117600348
--- Diff: distributedlog-core/src/main/java/org/apache/distributedlog/util/DLUtils.java ---
@@ -281,41 +281,89 @@ public static boolean isReservedStreamName(String name) {
}
/**
- * Validate the stream name.
+ * Validate the log name.
*
- * @param nameOfStream
- * name of stream
+ * @param logName
+ * name of log
* @throws InvalidStreamNameException
*/
- public static void validateName(String nameOfStream)
+ public static String validateAndNormalizeName(String logName)
throws InvalidStreamNameException {
- String reason = null;
- char chars[] = nameOfStream.toCharArray();
- char c;
- // validate the stream to see if meet zookeeper path's requirement
- for (int i = 0; i < chars.length; i++) {
- c = chars[i];
-
- if (c == 0) {
- reason = "null character not allowed @" + i;
- break;
- } else if (c == '/') {
- reason = "'/' 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 (isReservedStreamName(logName)) {
+ throw new InvalidStreamNameException(logName, "Log Name is reserved");
}
- if (null != reason) {
- throw new InvalidStreamNameException(nameOfStream, reason);
+
+ if (logName.charAt(0) == 47) {
+ validatePathName(logName);
+ return logName.substring(1);
+ } else {
+ validatePathName("/" + logName);
+ return logName;
}
- if (isReservedStreamName(nameOfStream)) {
- throw new InvalidStreamNameException(nameOfStream,
- "Stream Name is reserved");
+ }
+
+ private static void validatePathName(String logName) throws InvalidStreamNameException {
+ if (logName == null) {
+ throw new InvalidStreamNameException("Log name cannot be null");
+ } else if (logName.length() == 0) {
+ throw new InvalidStreamNameException("Log name length must be > 0");
+ } else if (logName.charAt(0) != 47) {
+ throw new InvalidStreamNameException("Log name must start with / character");
+ } else if (logName.length() != 1) {
+ if (logName.charAt(logName.length() - 1) == 47) {
+ throw new InvalidStreamNameException("Log name must not end with / character");
+ } else {
+ String reason = null;
+ char lastc = 47;
+ char[] chars = logName.toCharArray();
+
+ for (int i = 1; i < chars.length; ++i) {
+ char c = chars[i];
+ if (c == 0) {
+ reason = "null character not allowed @" + i;
+ break;
+ }
+
+ if (c == '<' || c == '>') {
+ reason = "< or > specified @" + i;
+ break;
+ }
+
+ if (c == ' ') {
--- End diff --
Is it needed other empty char like HT, VT, CR, NL('\t', '\r', '\n', '\x0b') ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-distributedlog pull request #130: DL-199: Be able to support files...
Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on a diff in the pull request:
https://github.com/apache/incubator-distributedlog/pull/130#discussion_r121057253
--- Diff: distributedlog-core/src/main/java/org/apache/distributedlog/util/Utils.java ---
@@ -635,4 +636,16 @@ public static String getParent(final String path) {
return path.substring(0, lastIndex);
}
+ /**
+ * Validate the provided {@code logName}.
+ *
+ * @param logName name of the log
+ * @throws InvalidStreamNameException if the name of the log is not valid.
+ */
+ public static void validateLogName(String logName) throws InvalidStreamNameException {
--- End diff --
oh i see. this seems to be redundant. going to remove it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-distributedlog issue #130: DL-199: Be able to support filesystem-p...
Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:
https://github.com/apache/incubator-distributedlog/pull/130
@leighst @yzang : mind reviewing this one?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-distributedlog pull request #130: DL-199: Be able to support files...
Posted by jiazhai <gi...@git.apache.org>.
Github user jiazhai commented on a diff in the pull request:
https://github.com/apache/incubator-distributedlog/pull/130#discussion_r117600036
--- Diff: distributedlog-core/src/main/java/org/apache/distributedlog/util/DLUtils.java ---
@@ -281,41 +281,89 @@ public static boolean isReservedStreamName(String name) {
}
/**
- * Validate the stream name.
+ * Validate the log name.
*
- * @param nameOfStream
- * name of stream
+ * @param logName
+ * name of log
* @throws InvalidStreamNameException
*/
- public static void validateName(String nameOfStream)
+ public static String validateAndNormalizeName(String logName)
throws InvalidStreamNameException {
- String reason = null;
- char chars[] = nameOfStream.toCharArray();
- char c;
- // validate the stream to see if meet zookeeper path's requirement
- for (int i = 0; i < chars.length; i++) {
- c = chars[i];
-
- if (c == 0) {
- reason = "null character not allowed @" + i;
- break;
- } else if (c == '/') {
- reason = "'/' 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 (isReservedStreamName(logName)) {
+ throw new InvalidStreamNameException(logName, "Log Name is reserved");
}
- if (null != reason) {
- throw new InvalidStreamNameException(nameOfStream, reason);
+
+ if (logName.charAt(0) == 47) {
--- End diff --
Is 47 intentional? How about replace 47 with '/' in this file?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-distributedlog pull request #130: DL-199: Be able to support files...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-distributedlog/pull/130
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-distributedlog pull request #130: DL-199: Be able to support files...
Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on a diff in the pull request:
https://github.com/apache/incubator-distributedlog/pull/130#discussion_r118790532
--- Diff: distributedlog-core/src/test/java/org/apache/distributedlog/TestBKDistributedLogNamespace.java ---
@@ -143,7 +172,7 @@ public void testInvalidStreamName() throws Exception {
}
try {
- namespace.openLog("/test2");
+ namespace.openLog("/ test2");
fail("should fail to create invalid stream /test2");
--- End diff --
done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---