This is an automated email from the ASF dual-hosted git repository.
baedke pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 4d29e3f783 OAK-10616: Make error messages from
o.a.j.o.namepath.JcrNameParser/Jc… (#1277)
4d29e3f783 is described below
commit 4d29e3f7838458f4a5627d93bc0f7831ff8bd6bf
Author: mbaedke <[email protected]>
AuthorDate: Tue Jan 23 13:51:46 2024 +0100
OAK-10616: Make error messages from o.a.j.o.namepath.JcrNameParser/Jc…
(#1277)
* OAK-10616: Make error messages from
o.a.j.o.namepath.JcrNameParser/JcrPathParser consistent and less misleading
Unified error messages from JcrPathParser and JcrNameParser.
---
.../jackrabbit/oak/namepath/JcrNameParser.java | 14 ++--
.../jackrabbit/oak/namepath/JcrPathParser.java | 85 ++++++++++++++--------
.../jackrabbit/oak/namepath/PathParserTest.java | 38 ++++------
3 files changed, 77 insertions(+), 60 deletions(-)
diff --git
a/oak-core-spi/src/main/java/org/apache/jackrabbit/oak/namepath/JcrNameParser.java
b/oak-core-spi/src/main/java/org/apache/jackrabbit/oak/namepath/JcrNameParser.java
index d0a6471d43..8322e5926a 100644
---
a/oak-core-spi/src/main/java/org/apache/jackrabbit/oak/namepath/JcrNameParser.java
+++
b/oak-core-spi/src/main/java/org/apache/jackrabbit/oak/namepath/JcrNameParser.java
@@ -80,7 +80,7 @@ public final class JcrNameParser {
// trivial check
int len = jcrName == null ? 0 : jcrName.length();
if (len == 0) {
- listener.error("Empty name");
+ listener.error("Empty name.");
return false;
}
if (".".equals(jcrName) || "..".equals(jcrName)) {
@@ -97,7 +97,7 @@ public final class JcrNameParser {
char c = jcrName.charAt(i);
if (c == ':') {
if (state == STATE_PREFIX_START) {
- listener.error("Prefix must not be empty");
+ listener.error("Prefix must not be empty.");
return false;
} else if (state == STATE_PREFIX) {
prefix = jcrName.substring(0, i);
@@ -109,17 +109,17 @@ public final class JcrNameParser {
} else if (state == STATE_URI) {
// ignore -> validation of uri later on.
} else {
- listener.error("'" + c + "' not allowed in name");
+ listener.error("'" + c + "' not allowed in name.");
return false;
}
} else if (c == '[' || c == ']' || c == '*' || c == '|') {
- listener.error("'" + c + "' not allowed in name");
+ listener.error("'" + c + "' not allowed in name.");
return false;
} else if (c == '/') {
if (state == STATE_URI_START) {
state = STATE_URI;
} else if (state != STATE_URI) {
- listener.error("'" + c + "' not allowed in name");
+ listener.error("'" + c + "' not allowed in name.");
return false;
}
} else if (c == '{') {
@@ -179,12 +179,12 @@ public final class JcrNameParser {
// take care of qualified jcrNames starting with '{' that are not
having
// a terminating '}' -> make sure there are no illegal characters
present.
if (state == STATE_URI && (jcrName.indexOf(':') > -1 ||
jcrName.indexOf('/') > -1)) {
- listener.error("Local name may not contain ':' nor '/'");
+ listener.error("Local name may not contain ':' nor '/'.");
return false;
}
if (nameStart == len || state == STATE_NAME_START) {
- listener.error("Local name must not be empty");
+ listener.error("Local name must not be empty.");
return false;
}
diff --git
a/oak-core-spi/src/main/java/org/apache/jackrabbit/oak/namepath/JcrPathParser.java
b/oak-core-spi/src/main/java/org/apache/jackrabbit/oak/namepath/JcrPathParser.java
index 553126b00c..614c6d7a1d 100644
---
a/oak-core-spi/src/main/java/org/apache/jackrabbit/oak/namepath/JcrPathParser.java
+++
b/oak-core-spi/src/main/java/org/apache/jackrabbit/oak/namepath/JcrPathParser.java
@@ -44,6 +44,42 @@ public final class JcrPathParser {
boolean parent();
}
+ private static final class PathAwareListener implements Listener {
+
+ private final Listener listener;
+ private final String jcrPath;
+
+ private PathAwareListener(Listener listener, String jcrPath) {
+ this.listener = listener;
+ this.jcrPath = jcrPath;
+ }
+
+ @Override
+ public void error(String message) {
+ listener.error("'" + jcrPath + "' is not a valid path. " +
message);
+ }
+
+ @Override
+ public boolean name(String name, int index) {
+ return listener.name(name, index);
+ }
+
+ @Override
+ public boolean root() {
+ return listener.root();
+ }
+
+ @Override
+ public boolean current() {
+ return listener.current();
+ }
+
+ @Override
+ public boolean parent() {
+ return listener.parent();
+ }
+ }
+
public static boolean parse(String jcrPath, Listener listener) {
// check for length
int len = jcrPath == null ? 0 : jcrPath.length();
@@ -77,6 +113,8 @@ public final class JcrPathParser {
int index = 0;
boolean wasSlash = false;
+ final PathAwareListener pathAwareListener = new
PathAwareListener(listener, jcrPath);
+
while (pos <= len) {
char c = pos == len ? EOF : jcrPath.charAt(pos);
pos++;
@@ -85,8 +123,7 @@ public final class JcrPathParser {
case '/':
case EOF:
if (state == STATE_PREFIX_START && c != EOF) {
- listener.error('\'' + jcrPath + "' is not a valid
path. " +
- "double slash '//' not allowed.");
+ pathAwareListener.error("Double slash '//' not
allowed.");
return false;
}
if (state == STATE_PREFIX
@@ -97,14 +134,13 @@ public final class JcrPathParser {
// eof path element
if (name == null) {
if (wasSlash) {
- listener.error('\'' + jcrPath + "' is not a
valid path: " +
- "Trailing slashes not allowed in
prefixes and names.");
+ pathAwareListener.error("Trailing slashes not
allowed in prefixes and names.");
return false;
}
name = jcrPath.substring(lastPos, pos - 1);
}
- if (!JcrNameParser.parse(name, listener, index)) {
+ if (!JcrNameParser.parse(name, pathAwareListener,
index)) {
return false;
}
state = STATE_PREFIX_START;
@@ -112,25 +148,24 @@ public final class JcrPathParser {
name = null;
index = 0;
} else if (state == STATE_DOT) {
- if (!listener.current()) {
+ if (!pathAwareListener.current()) {
return false;
}
lastPos = pos;
state = STATE_PREFIX_START;
} else if (state == STATE_DOTDOT) {
- if (!listener.parent()) {
+ if (!pathAwareListener.parent()) {
return false;
}
lastPos = pos;
state = STATE_PREFIX_START;
} else if (state != STATE_URI
&& !(state == STATE_PREFIX_START && c == EOF)) {
// ignore trailing slash
- listener.error('\'' + jcrPath + "' is not a valid
path. '" + c +
- "' not a valid name character.");
+ pathAwareListener.error("'" + c + "' not allowed in
name.");
return false;
} else if (state == STATE_URI && c == EOF) {
// we reached EOF without finding the closing curly
brace '}'
- listener.error('\'' + jcrPath + "' is not a valid
path. Missing '}'.");
+ pathAwareListener.error("Missing '}'.");
return false;
}
break;
@@ -143,28 +178,24 @@ public final class JcrPathParser {
} else if (state == STATE_DOTDOT) {
state = STATE_PREFIX;
} else if (state == STATE_INDEX_END) {
- listener.error('\'' + jcrPath + "' is not a valid
path. '" + c +
- "' not valid after index. '/' expected.");
+ pathAwareListener.error("'" + c + "' not valid after
index. '/' expected.");
return false;
}
break;
case ':':
if (state == STATE_PREFIX_START) {
- listener.error('\'' + jcrPath + "' is not a valid
path. Prefix " +
- "must not be empty");
+ pathAwareListener.error("Prefix must not be empty.");
return false;
} else if (state == STATE_PREFIX) {
if (wasSlash) {
- listener.error('\'' + jcrPath + "' is not a valid
path: " +
- "Trailing slashes not allowed in prefixes
and names.");
+ pathAwareListener.error("Trailing slashes not
allowed in prefixes and names.");
return false;
}
state = STATE_NAME_START;
// don't reset the lastPos/pos since prefix+name are
passed together to the NameResolver
} else if (state != STATE_URI) {
- listener.error('\'' + jcrPath + "' is not a valid
path. '" + c +
- "' not valid name character");
+ pathAwareListener.error("'" + c + "' not allowed in
name.");
return false;
}
break;
@@ -172,8 +203,7 @@ public final class JcrPathParser {
case '[':
if (state == STATE_PREFIX || state == STATE_NAME) {
if (wasSlash) {
- listener.error('\'' + jcrPath + "' is not a valid
path: " +
- "Trailing slashes not allowed in prefixes
and names.");
+ pathAwareListener.error("Trailing slashes not
allowed in prefixes and names.");
return false;
}
state = STATE_INDEX;
@@ -187,28 +217,24 @@ public final class JcrPathParser {
try {
index =
Integer.parseInt(jcrPath.substring(lastPos, pos - 1));
} catch (NumberFormatException e) {
- listener.error('\'' + jcrPath + "' is not a valid
path. " +
- "NumberFormatException in index: " +
+ pathAwareListener.error("NumberFormatException in
index: " +
jcrPath.substring(lastPos, pos - 1));
return false;
}
if (index < 0) {
- listener.error('\'' + jcrPath + "' is not a valid
path. " +
- "Index number invalid: " + index);
+ pathAwareListener.error("Index number invalid: " +
index);
return false;
}
state = STATE_INDEX_END;
} else {
- listener.error('\'' + jcrPath + "' is not a valid
path. '" + c +
- "' not a valid name character.");
+ pathAwareListener.error("'" + c + "' not allowed in
name.");
return false;
}
break;
case '*':
case '|':
- listener.error('\'' + jcrPath + "' is not a valid path. '"
+ c +
- "' not a valid name character.");
+ pathAwareListener.error("'" + c + "' not allowed in
name.");
return false;
case '{':
if (state == STATE_PREFIX_START && lastPos == pos-1) {
@@ -234,8 +260,7 @@ public final class JcrPathParser {
} else if (state == STATE_NAME_START) {
state = STATE_NAME;
} else if (state == STATE_INDEX_END) {
- listener.error('\'' + jcrPath + "' is not a valid
path. '" + c +
- "' not valid after index. '/' expected.");
+ pathAwareListener.error("'" + c + "' not valid after
index. '/' expected.");
return false;
}
}
diff --git
a/oak-core-spi/src/test/java/org/apache/jackrabbit/oak/namepath/PathParserTest.java
b/oak-core-spi/src/test/java/org/apache/jackrabbit/oak/namepath/PathParserTest.java
index 46fcdaea5e..edf24e47ba 100755
---
a/oak-core-spi/src/test/java/org/apache/jackrabbit/oak/namepath/PathParserTest.java
+++
b/oak-core-spi/src/test/java/org/apache/jackrabbit/oak/namepath/PathParserTest.java
@@ -95,9 +95,6 @@ public class PathParserTest {
private static final ParserCallbackResult CALLBACKRESULT_ROOT = new
ParserCallbackResult(ParserCallbackResultType.CALLBACK_ROOT, null, 0);
private static final ParserCallbackResult CALLBACKRESULT_CURRENT = new
ParserCallbackResult(ParserCallbackResultType.CALLBACK_CURRENT, null, 0);
private static final ParserCallbackResult CALLBACKRESULT_PARENT = new
ParserCallbackResult(ParserCallbackResultType.CALLBACK_PARENT, null, 0);
- private static final ParserCallbackResult CALLBACKRESULT_ERROR_ANY = new
ParserCallbackResult(ParserCallbackResultType.CALLBACK_ERROR, null, 0);
- private static final ParserCallbackResult CALLBACKRESULT_NAME_ANY = new
ParserCallbackResult(ParserCallbackResultType.CALLBACK_NAME, null, 0);
-
private static ParserCallbackResult CALLBACKRESULT_NAME(String name) {
return new
ParserCallbackResult(ParserCallbackResultType.CALLBACK_NAME, name, 0);
}
@@ -167,7 +164,7 @@ public class PathParserTest {
CALLBACKRESULT_CURRENT,
CALLBACKRESULT_PARENT,
CALLBACKRESULT_NAME("c", 1),
- CALLBACKRESULT_ERROR("'" + path + "' is not a valid path. ']'
not a valid name character.")
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path, ']'))
);
assertFalse(JcrPathParser.validate(path));
@@ -195,7 +192,7 @@ public class PathParserTest {
public void testUnexpectedOpeningSquareBracket() throws
RepositoryException {
String path = "[";
TestListener listener = new TestListener(
- CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName('['))
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path,'['))
);
assertFalse(JcrPathParser.validate(path));
assertFalse(JcrPathParser.parse(path, listener));
@@ -204,7 +201,7 @@ public class PathParserTest {
path = "/[";
listener = new TestListener(
CALLBACKRESULT_ROOT,
- CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName('['))
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path,'['))
);
assertFalse(JcrPathParser.validate(path));
assertFalse(JcrPathParser.parse(path, listener));
@@ -213,7 +210,7 @@ public class PathParserTest {
path = "./[";
listener = new TestListener(
CALLBACKRESULT_CURRENT,
- CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName('['))
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path,'['))
);
assertFalse(JcrPathParser.validate(path));
assertFalse(JcrPathParser.parse(path, listener));
@@ -222,7 +219,7 @@ public class PathParserTest {
path = "../[";
listener = new TestListener(
CALLBACKRESULT_PARENT,
- CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName('['))
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path,'['))
);
assertFalse(JcrPathParser.validate(path));
assertFalse(JcrPathParser.parse(path, listener));
@@ -230,7 +227,7 @@ public class PathParserTest {
path = ".[";
listener = new TestListener(
- CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName('['))
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path,'['))
);
assertFalse(JcrPathParser.validate(path));
assertFalse(JcrPathParser.parse(path, listener));
@@ -238,7 +235,7 @@ public class PathParserTest {
path = "..[";
listener = new TestListener(
- CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName('['))
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path,'['))
);
assertFalse(JcrPathParser.validate(path));
assertFalse(JcrPathParser.parse(path, listener));
@@ -246,7 +243,7 @@ public class PathParserTest {
path = "{[}";
listener = new TestListener(
- CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName('['))
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path,'['))
);
assertFalse(JcrPathParser.validate(path));
assertFalse(JcrPathParser.parse(path, listener));
@@ -280,7 +277,7 @@ public class PathParserTest {
public void testUnxepectedClosingSquareBracket() throws
RepositoryException {
String path = "]";
TestListener listener = new TestListener(
- CALLBACKRESULT_ERROR(errorInvalidCharacterInName(path, ']'))
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path, ']'))
);
assertFalse(JcrPathParser.validate(path));
assertFalse(JcrPathParser.parse(path, listener));
@@ -289,7 +286,7 @@ public class PathParserTest {
path = "/]";
listener = new TestListener(
CALLBACKRESULT_ROOT,
- CALLBACKRESULT_ERROR(errorInvalidCharacterInName(path, ']'))
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path, ']'))
);
assertFalse(JcrPathParser.validate(path));
assertFalse(JcrPathParser.parse(path, listener));
@@ -298,7 +295,7 @@ public class PathParserTest {
path = ".]";
listener = new TestListener(
//TODO improve error message?
- CALLBACKRESULT_ERROR(errorInvalidCharacterInName(path, ']'))
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path, ']'))
);
assertFalse(JcrPathParser.validate(path));
assertFalse(JcrPathParser.parse(path, listener));
@@ -307,7 +304,7 @@ public class PathParserTest {
path = "..]";
listener = new TestListener(
//TODO improve error message?
- CALLBACKRESULT_ERROR(errorInvalidCharacterInName(path, ']'))
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path, ']'))
);
assertFalse(JcrPathParser.validate(path));
assertFalse(JcrPathParser.parse(path, listener));
@@ -315,7 +312,7 @@ public class PathParserTest {
path = "{]}";
listener = new TestListener(
- CALLBACKRESULT_ERROR(errorInvalidCharacterInName(path, ']'))
+ CALLBACKRESULT_ERROR(errorCharacterNotAllowedInName(path, ']'))
);
assertFalse(JcrPathParser.validate(path));
assertFalse(JcrPathParser.parse(path, listener));
@@ -353,13 +350,8 @@ public class PathParserTest {
listener.evaluate();
}
- //TODO replace with #errorInvalidCharacterInName() to make error messages
consistent?
- private static String errorCharacterNotAllowedInName(char c) {
- return "'" + c + "' not allowed in name";
- }
-
- private static String errorInvalidCharacterInName(String path, char c) {
- return "'" + path + "' is not a valid path. ']' not a valid name
character.";
+ private static String errorCharacterNotAllowedInName(String path, char c) {
+ return "'" + path + "' is not a valid path. '" + c + "' not allowed in
name.";
}
private static String errorClosingQuareBracketExpected(String path) {
