Don't rely on ConfigurationPropertyName exceptions

Update `ConfigurationPropertySourcesPropertySource` to no longer use
`try/catch` when checking for valid names. A new `isValid` method has
been introduced to `ConfigurationPropertyName` which is offers better
performance.

Fixes gh-9058
pull/9050/merge
Phillip Webb 8 years ago
parent 8b1625b41d
commit 5ae1798ec5

@ -226,6 +226,45 @@ public final class ConfigurationPropertyName
return this; return this;
} }
/**
* Returns if the given name is valid. If this method returns {@code true} then the
* name may be used with {@link #of(String)} without throwing an exception.
* @param name the name to test
* @return {@code true} if the name is valid
*/
public static boolean isValid(String name) {
if (name == null) {
return false;
}
boolean indexed = false;
int charIndex = 0;
for (int i = 0; i < name.length(); i++) {
char ch = name.charAt(i);
if (!indexed) {
if (ch == '[') {
indexed = true;
charIndex = 1;
}
else if (ch == '.') {
charIndex = 0;
}
else {
if (!Element.isValid(charIndex, ch)) {
return false;
}
charIndex++;
}
}
else {
if (ch == ']') {
indexed = false;
charIndex = 0;
}
}
}
return true;
}
/** /**
* Return a {@link ConfigurationPropertyName} for the specified string. * Return a {@link ConfigurationPropertyName} for the specified string.
* @param name the source name * @param name the source name
@ -335,7 +374,27 @@ public final class ConfigurationPropertyName
return this.value[form.ordinal()]; return this.value[form.ordinal()];
} }
public static boolean isIndexed(String value) { static boolean isValid(String value) {
if (!isIndexed(value)) {
for (int i = 0; i < value.length(); i++) {
if (!isValid(i, value.charAt(i))) {
return false;
}
}
}
return true;
}
static boolean isValid(int index, char ch) {
boolean isAlpha = ch >= 'a' && ch <= 'z';
boolean isNumeric = ch >= '0' && ch <= '9';
if (index == 0) {
return isAlpha;
}
return isAlpha || isNumeric || ch == '-';
}
private static boolean isIndexed(String value) {
return value.startsWith("[") && value.endsWith("]"); return value.startsWith("[") && value.endsWith("]");
} }

@ -142,7 +142,6 @@ class ConfigurationPropertyNameBuilder {
* An processor that will be applied to element values. Can be used to manipulate or * An processor that will be applied to element values. Can be used to manipulate or
* restrict the values that are used. * restrict the values that are used.
*/ */
@FunctionalInterface
public interface ElementValueProcessor { public interface ElementValueProcessor {
/** /**
@ -169,16 +168,9 @@ class ConfigurationPropertyNameBuilder {
default ElementValueProcessor withValidName() { default ElementValueProcessor withValidName() {
return (value) -> { return (value) -> {
value = apply(value); value = apply(value);
if (!Element.isIndexed(value)) { if (!Element.isValid(value)) {
for (int i = 0; i < value.length(); i++) { throw new IllegalArgumentException(
char ch = value.charAt(i); "Element value '" + value + "' is not valid");
boolean isAlpha = ch >= 'a' && ch <= 'z';
boolean isNumeric = ch >= '0' && ch <= '9';
if (i == 0 && !isAlpha || !(isAlpha || isNumeric || ch == '-')) {
throw new IllegalArgumentException(
"Element value '" + value + "' is not valid");
}
}
} }
return value; return value;
}; };

@ -56,11 +56,13 @@ class ConfigurationPropertySourcesPropertySource
private ConfigurationProperty findConfigurationProperty(String name) { private ConfigurationProperty findConfigurationProperty(String name) {
try { try {
return findConfigurationProperty(ConfigurationPropertyName.of(name)); if (ConfigurationPropertyName.isValid(name)) {
return findConfigurationProperty(ConfigurationPropertyName.of(name));
}
} }
catch (Exception ex) { catch (Exception ex) {
return null;
} }
return null;
} }
private ConfigurationProperty findConfigurationProperty( private ConfigurationProperty findConfigurationProperty(

@ -332,4 +332,24 @@ public class ConfigurationPropertyNameTests {
assertThat(name.append("foo").toString()).isEqualTo("foo"); assertThat(name.append("foo").toString()).isEqualTo("foo");
} }
@Test
public void isValidWhenValidShouldReturnTrue() throws Exception {
assertThat(ConfigurationPropertyName.isValid("")).isTrue();
assertThat(ConfigurationPropertyName.isValid("foo")).isTrue();
assertThat(ConfigurationPropertyName.isValid("foo.bar")).isTrue();
assertThat(ConfigurationPropertyName.isValid("foo[0]")).isTrue();
assertThat(ConfigurationPropertyName.isValid("foo[0].baz")).isTrue();
assertThat(ConfigurationPropertyName.isValid("foo.b1")).isTrue();
assertThat(ConfigurationPropertyName.isValid("foo.b-a-r")).isTrue();
assertThat(ConfigurationPropertyName.isValid("foo[FooBar].baz")).isTrue();
}
@Test
public void isValidWhenNotValidShouldReturnFalse() throws Exception {
assertThat(ConfigurationPropertyName.isValid(null)).isFalse();
assertThat(ConfigurationPropertyName.isValid("1foo")).isFalse();
assertThat(ConfigurationPropertyName.isValid("FooBar")).isFalse();
assertThat(ConfigurationPropertyName.isValid("foo!bar")).isFalse();
}
} }

Loading…
Cancel
Save