JCR-4913: spi-commons: improve error messages for org.apache.jackrabbit.spi.commons.conversion.NameParser.parse (also cleanup tests a bit)
git-svn-id: https://svn.apache.org/repos/asf/jackrabbit/trunk@1908408 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/conversion/NameParser.java b/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/conversion/NameParser.java
index 9eabfab..2768c67 100644
--- a/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/conversion/NameParser.java
+++ b/jackrabbit-spi-commons/src/main/java/org/apache/jackrabbit/spi/commons/conversion/NameParser.java
@@ -80,27 +80,29 @@
}
prefix = jcrName.substring(0, i);
if (!XMLChar.isValidNCName(prefix)) {
- throw new IllegalNameException("Invalid name prefix: "+ prefix);
+ throw new IllegalNameException("Invalid name prefix: " + prefix);
}
state = STATE_NAME_START;
} else if (state == STATE_URI) {
// ignore -> validation of uri later on.
} else {
- throw new IllegalNameException("'" + c + "' not allowed in name");
+ throw new IllegalNameException(asDisplayableString(c) + " not allowed in name");
}
trailingSpaces = false;
} else if (c == ' ') {
if (state == STATE_PREFIX_START || state == STATE_NAME_START) {
- throw new IllegalNameException("'" + c + "' not valid name start");
+ throw new IllegalNameException(asDisplayableString(c) + " not valid name start");
}
trailingSpaces = true;
- } else if (Character.isWhitespace(c) || c == '[' || c == ']' || c == '*' || c == '|') {
- throw new IllegalNameException("'" + c + "' not allowed in name");
+ } else if (c == '[' || c == ']' || c == '*' || c == '|') {
+ throw new IllegalNameException(asDisplayableString(c) + " not allowed in name");
+ } else if (Character.isWhitespace(c)) {
+ throw new IllegalNameException("Whitespace character " + asDisplayableString(c) + " not allowed in name");
} else if (c == '/') {
if (state == STATE_URI_START) {
state = STATE_URI;
} else if (state != STATE_URI) {
- throw new IllegalNameException("'" + c + "' not allowed in name");
+ throw new IllegalNameException(asDisplayableString(c) + " not allowed in name");
}
trailingSpaces = false;
} else if (c == '{') {
@@ -191,6 +193,14 @@
return factory.create(uri, localName);
}
+ private static String asDisplayableString(char c) {
+ if (Character.isWhitespace(c)) {
+ return String.format("U+%04x", (int)c);
+ } else {
+ return "'" + c + "'";
+ }
+ }
+
/**
* Parses an array of <code>jcrName</code> and returns the respective
* array of <code>Name</code>.
diff --git a/jackrabbit-spi-commons/src/test/java/org/apache/jackrabbit/spi/commons/conversion/NameParserTest.java b/jackrabbit-spi-commons/src/test/java/org/apache/jackrabbit/spi/commons/conversion/NameParserTest.java
index e45b17d..b7d24c2 100644
--- a/jackrabbit-spi-commons/src/test/java/org/apache/jackrabbit/spi/commons/conversion/NameParserTest.java
+++ b/jackrabbit-spi-commons/src/test/java/org/apache/jackrabbit/spi/commons/conversion/NameParserTest.java
@@ -26,7 +26,6 @@
import javax.jcr.NamespaceException;
import java.util.List;
import java.util.ArrayList;
-import java.util.Iterator;
/**
* <code>NameParserTest</code>...
@@ -92,7 +91,7 @@
public void testExpandedJcrNames() throws NamespaceException, IllegalNameException {
NamespaceResolver resolver = new TestNamespaceResolver();
- List<String[]> valid = new ArrayList<String[]>();
+ List<String[]> valid = new ArrayList<>();
// valid qualified jcr-names:
// String-array consisting of { jcrName , uri , localName }
valid.add(new String[] {"abc:{c}", "abc", "{c}"});
@@ -125,8 +124,7 @@
valid.add(new String[] {"{abc:}def", "abc:", "def"});
valid.add(new String[] {"{}abc", "", "abc"});
- for (Object aValid : valid) {
- String[] strs = (String[]) aValid;
+ for (String[] strs : valid) {
try {
Name n = NameParser.parse(strs[0], resolver, factory);
assertEquals("URI mismatch", strs[1], n.getNamespaceURI());
@@ -158,8 +156,7 @@
invalid.add("{http://jackrabbit.apache.org}");
invalid.add("{}");
- for (Object anInvalid : invalid) {
- String jcrName = (String) anInvalid;
+ for (String jcrName : invalid) {
try {
NameParser.parse(jcrName, resolver, factory);
fail("Parsing '" + jcrName + "' should fail. Not a valid jcr name.");
@@ -231,8 +228,7 @@
invalid.add("{/jackrabbit/a/b/c}abc");
- for (Object anInvalid : invalid) {
- String jcrName = (String) anInvalid;
+ for (String jcrName : invalid) {
try {
NameParser.checkFormat(jcrName);
fail("Checking format of '" + jcrName + "' should fail. Not a valid jcr name.");
@@ -241,7 +237,16 @@
}
}
}
-
+
+ public void testMessage() {
+ try {
+ NameParser.checkFormat("horizontal\ttab");
+ fail("should fail with IllegalNameException");
+ } catch (IllegalNameException ex) {
+ assertTrue("message should contain 'U+0009'", ex.getMessage().indexOf("U+0009") >= 0);
+ }
+ }
+
/**
* Dummy NamespaceResolver that only knows the empty namespace and
* namespaces containing either 'jackrabbit' or 'abc'. Used to test
diff --git a/jackrabbit-spi-commons/src/test/java/org/apache/jackrabbit/spi/commons/name/JcrName.java b/jackrabbit-spi-commons/src/test/java/org/apache/jackrabbit/spi/commons/name/JcrName.java
index bed4ec1..2aabebc 100644
--- a/jackrabbit-spi-commons/src/test/java/org/apache/jackrabbit/spi/commons/name/JcrName.java
+++ b/jackrabbit-spi-commons/src/test/java/org/apache/jackrabbit/spi/commons/name/JcrName.java
@@ -17,6 +17,7 @@
package org.apache.jackrabbit.spi.commons.name;
import java.util.ArrayList;
+import java.util.List;
/**
* <code>JcrName</code>...
@@ -28,7 +29,7 @@
public final String name;
// create tests
- private static ArrayList list = new ArrayList();
+ private static List<JcrName> list = new ArrayList<>();
static {
// valid names
list.add(new JcrName("a", "", "a"));
@@ -48,7 +49,7 @@
list.add(new JcrName("a\"", "", "a\"")); // double quote
list.add(new JcrName("\"a", "", "\"a"));
list.add(new JcrName("ab\"c", "", "ab\"c"));
- list.add(new JcrName("prefix:ab\"c", "prefix", "ab\"c"));
+ list.add(new JcrName("prefix:ab\"c", "prefix", "ab\"c"));
// expanded names
list.add(new JcrName("{}a", "", "a"));
@@ -89,9 +90,13 @@
list.add(new JcrName("|name"));
list.add(new JcrName("name|"));
list.add(new JcrName("prefix:name|name"));
+
+ // invalid whitespace
+ list.add(new JcrName("horizontal\ttab"));
+ list.add(new JcrName("line\nfeed"));
}
- private static JcrName[] tests = (JcrName[]) list.toArray(new JcrName[list.size()]);
+ private static JcrName[] tests = list.toArray(new JcrName[list.size()]);
public static JcrName[] getTests() {
return tests;
@@ -108,7 +113,7 @@
}
public boolean isValid() {
- return name!=null;
+ return name != null;
}
public String toString() {
