You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2018/09/04 08:23:57 UTC
[8/9] brooklyn-server git commit: address code review comments
address code review comments
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/bae72fc4
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/bae72fc4
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/bae72fc4
Branch: refs/heads/master
Commit: bae72fc4ed1bc1813a4014e71955f51dad1e1994
Parents: 8e48531
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Mon Sep 3 15:22:09 2018 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Mon Sep 3 15:26:18 2018 +0100
----------------------------------------------------------------------
.../brooklyn/core/config/ConfigConstraints.java | 6 +-
.../brooklyn/util/core/task/ValueResolver.java | 2 +-
.../brooklyn/util/text/StringEscapes.java | 17 +-
.../brooklyn/util/text/StringEscapesTest.java | 175 +++++++++++--------
4 files changed, 119 insertions(+), 81 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bae72fc4/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java
index 91f4eab..17b7c9d 100644
--- a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java
+++ b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java
@@ -89,7 +89,7 @@ public abstract class ConfigConstraints<T extends BrooklynObject> {
}
private static <T> void assertValid(ConfigConstraints<?> constrants, Object context, ConfigKey<T> key, T value) {
- ReferenceWithError<Predicate<?>> validity = constrants.checkValueValid(key, value);
+ ReferenceWithError<Predicate<?>> validity = constrants.validateValue(key, value);
if (validity.hasError()) {
throw new ConstraintViolationException("Invalid value for " + key + " on " + context + " (" + value + "); it should satisfy "+validity.getWithoutError());
}
@@ -154,12 +154,12 @@ public abstract class ConfigConstraints<T extends BrooklynObject> {
}
<V> boolean isValueValid(ConfigKey<V> configKey, V value) {
- return !checkValueValid(configKey, value).hasError();
+ return !validateValue(configKey, value).hasError();
}
/** returns reference to null without error if valid; otherwise returns reference to predicate and a good error message */
@SuppressWarnings("unchecked")
- <V> ReferenceWithError<Predicate<?>> checkValueValid(ConfigKey<V> configKey, V value) {
+ <V> ReferenceWithError<Predicate<?>> validateValue(ConfigKey<V> configKey, V value) {
try {
Predicate<? super V> po = configKey.getConstraint();
boolean valid;
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bae72fc4/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java b/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java
index f39af09..b1c44c3 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java
@@ -265,7 +265,7 @@ public class ValueResolver<T> implements DeferredSupplier<T>, Iterable<Maybe<Obj
* if the second argument is true, the type specified here is used against non-map/iterable items
* inside maps and iterables encountered. if false, any generic signature on the supplied type
* is traversed to match contained items. if null (default), it is inferred from the type,
- * those with generics mean true, and those without mean false.
+ * those with generics imply false, and those without imply true.
*
* see {@link Tasks#resolveDeepValue(Object, Class, ExecutionContext, String)} and
* {@link Tasks#resolveDeepValueExactly(Object, TypeToken, ExecutionContext, String)}. */
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bae72fc4/utils/common/src/main/java/org/apache/brooklyn/util/text/StringEscapes.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/text/StringEscapes.java b/utils/common/src/main/java/org/apache/brooklyn/util/text/StringEscapes.java
index f3df2b2..98e183e 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/text/StringEscapes.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/text/StringEscapes.java
@@ -321,7 +321,7 @@ public class StringEscapes {
}
/** @deprecated since 1.0.0, use {@link #unwrapJsonishListStringIfPossible(String)} (old semantics)
- * or {@link #unwrapJsonishListStringIfPossible(String)} (improved) */
+ * or {@link #tryUnwrapJsonishList(String)} (improved) */
public static List<String> unwrapJsonishListIfPossible(String input) {
return unwrapJsonishListStringIfPossible(input);
}
@@ -335,10 +335,10 @@ public class StringEscapes {
* <ll> 2) if not of form <code>[ X ]</code>, wrap in brackets and parse as YAML,
* and if that succeeds and is a list, return the result.
* <li> 3) otherwise, expect comma-separated tokens which after trimming are of the form "A" or B,
- * where "A" is a valid Java string or C is any string not starting with '
- * and not containing " or ,. return the list of those tokens, where A and B
- * are their string value, and C as a primitive if it is a number or boolean or null,
- * or else a string, including the empty string if empty.
+ * where "A" is a valid Java string or B is any string not containing any of the chars <code>",.</code>
+ * and not starting with <code>'</code>, and returns the list of those tokens, where A is
+ * returned as its string value, and B as a primitive if it is a number or boolean or null,
+ * or else a string (including the empty string if empty)
* <li> 4) if such tokens are not found, return {@link Maybe#absent()}.
* <p>
* @see #unwrapOptionallyQuotedJavaStringList(String)
@@ -362,7 +362,10 @@ public class StringEscapes {
List<Object> result = (List<Object>)r;
return Maybe.of(result);
}
- } catch (Exception e) {}
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ // Otherwise ignore; logic below decides whether to return absent or to keep trying
+ }
if (inputT.startsWith("[")) {
// if supplied as yaml, don't allow failures
return Maybe.absent("Supplied format looked like YAML but could not parse as YAML");
@@ -398,7 +401,7 @@ public class StringEscapes {
String w = m.group(1);
ri = w;
- if (w.matches("[0-9]*.[0-9]+")) {
+ if (w.matches("[0-9]*\\.[0-9]+")) {
try {
ri = Double.parseDouble(w);
} catch (Exception e) {}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bae72fc4/utils/common/src/test/java/org/apache/brooklyn/util/text/StringEscapesTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/text/StringEscapesTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/text/StringEscapesTest.java
index 8739484..613bc6c 100644
--- a/utils/common/src/test/java/org/apache/brooklyn/util/text/StringEscapesTest.java
+++ b/utils/common/src/test/java/org/apache/brooklyn/util/text/StringEscapesTest.java
@@ -87,93 +87,126 @@ public class StringEscapesTest {
@SuppressWarnings("deprecation")
@Test
public void testJavaListDeprecated() {
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.unwrapQuotedJavaStringList("\"hello\", \"world\"", ","));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapQuotedJavaStringList("\"hello\", \"world\"", ","),
+ MutableList.of("hello", "world"));
try {
JavaStringEscapes.unwrapQuotedJavaStringList("\"hello\", world", ",");
Assert.fail("Should have thrown");
} catch (Exception e) { /* expected */ }
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.unwrapJsonishListIfPossible("\"hello\", \"world\""));
- Assert.assertEquals(MutableList.of("hello"),
- JavaStringEscapes.unwrapJsonishListIfPossible("hello"));
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.unwrapJsonishListIfPossible("hello, world"));
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.unwrapJsonishListIfPossible("\"hello\", world"));
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.unwrapJsonishListIfPossible("[ \"hello\", world ]"));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListIfPossible("\"hello\", \"world\""),
+ MutableList.of("hello", "world"));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListIfPossible("hello"),
+ MutableList.of("hello"));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListIfPossible("hello, world"),
+ MutableList.of("hello", "world"));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListIfPossible("\"hello\", world"),
+ MutableList.of("hello", "world"));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListIfPossible("[ \"hello\", world ]"),
+ MutableList.of("hello", "world"));
// if can't parse e.g. because contains double quote, then returns original string as single element list
- Assert.assertEquals(MutableList.of("hello\", \"world\""),
- JavaStringEscapes.unwrapJsonishListIfPossible("hello\", \"world\""));
- Assert.assertEquals(MutableList.of(),
- JavaStringEscapes.unwrapJsonishListIfPossible(" "));
- Assert.assertEquals(MutableList.of(""),
- JavaStringEscapes.unwrapJsonishListIfPossible("\"\""));
- Assert.assertEquals(MutableList.of("x"),
- JavaStringEscapes.unwrapJsonishListIfPossible(",,x,"));
- Assert.assertEquals(MutableList.of("","x",""),
- JavaStringEscapes.unwrapJsonishListIfPossible("\"\",,x,\"\""));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListIfPossible("hello\", \"world\""),
+ MutableList.of("hello\", \"world\""));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListIfPossible(" "),
+ MutableList.of());
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListIfPossible("\"\""),
+ MutableList.of(""));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListIfPossible(",,x,"),
+ MutableList.of("x"));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListIfPossible("\"\",,x,\"\""),
+ MutableList.of("","x",""));
}
@Test
public void testJavaListString() {
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.unwrapQuotedJavaStringList("\"hello\", \"world\"", ","));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapQuotedJavaStringList("\"hello\", \"world\"", ","),
+ MutableList.of("hello", "world"));
try {
JavaStringEscapes.unwrapQuotedJavaStringList("\"hello\", world", ",");
Assert.fail("Should have thrown");
} catch (Exception e) { /* expected */ }
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.unwrapJsonishListStringIfPossible("\"hello\", \"world\""));
- Assert.assertEquals(MutableList.of("hello"),
- JavaStringEscapes.unwrapJsonishListStringIfPossible("hello"));
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.unwrapJsonishListStringIfPossible("hello, world"));
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.unwrapJsonishListStringIfPossible("\"hello\", world"));
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.unwrapJsonishListStringIfPossible("[ \"hello\", world ]"));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListStringIfPossible("\"hello\", \"world\""),
+ MutableList.of("hello", "world"));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListStringIfPossible("hello"),
+ MutableList.of("hello"));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListStringIfPossible("hello, world"),
+ MutableList.of("hello", "world"));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListStringIfPossible("\"hello\", world"),
+ MutableList.of("hello", "world"));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListStringIfPossible("[ \"hello\", world ]"),
+ MutableList.of("hello", "world"));
// if can't parse e.g. because contains double quote, then returns original string as single element list
- Assert.assertEquals(MutableList.of("hello\", \"world\""),
- JavaStringEscapes.unwrapJsonishListStringIfPossible("hello\", \"world\""));
- Assert.assertEquals(MutableList.of(),
- JavaStringEscapes.unwrapJsonishListStringIfPossible(" "));
- Assert.assertEquals(MutableList.of(""),
- JavaStringEscapes.unwrapJsonishListStringIfPossible("\"\""));
- Assert.assertEquals(MutableList.of("x"),
- JavaStringEscapes.unwrapJsonishListStringIfPossible(",,x,"));
- Assert.assertEquals(MutableList.of("","x",""),
- JavaStringEscapes.unwrapJsonishListStringIfPossible("\"\",,x,\"\""));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListStringIfPossible("hello\", \"world\""),
+ MutableList.of("hello\", \"world\""));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListStringIfPossible(" "),
+ MutableList.of());
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListStringIfPossible("\"\""),
+ MutableList.of(""));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListStringIfPossible(",,x,"),
+ MutableList.of("x"));
+ Assert.assertEquals(
+ JavaStringEscapes.unwrapJsonishListStringIfPossible("\"\",,x,\"\""),
+ MutableList.of("","x",""));
}
@Test
public void testJavaListObject() {
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.tryUnwrapJsonishList("\"hello\", \"world\"").get());
- Assert.assertEquals(MutableList.of("hello"),
- JavaStringEscapes.tryUnwrapJsonishList("hello").get());
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.tryUnwrapJsonishList("hello, world").get());
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.tryUnwrapJsonishList("\"hello\", world").get());
- Assert.assertEquals(MutableList.of("hello", "world"),
- JavaStringEscapes.tryUnwrapJsonishList("[ \"hello\", world ]").get());
- Assert.assertEquals(MutableList.of(),
- JavaStringEscapes.tryUnwrapJsonishList(" ").get());
- Assert.assertEquals(MutableList.of(""),
- JavaStringEscapes.tryUnwrapJsonishList("\"\"").get());
- Assert.assertEquals(MutableList.of("","","x",""),
- JavaStringEscapes.tryUnwrapJsonishList(",,x,").get());
- Assert.assertEquals(MutableList.of("","","x",""),
- JavaStringEscapes.tryUnwrapJsonishList("\"\",,x,\"\"").get());
- Assert.assertEquals(MutableList.<Object>of(MutableMap.of("a", 1),MutableMap.of("b", 2)),
- JavaStringEscapes.tryUnwrapJsonishList("[ a : 1, b : 2 ]").get());
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList("\"hello\", \"world\"").get(),
+ MutableList.of("hello", "world"));
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList("hello").get(),
+ MutableList.of("hello"));
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList("hello, world").get(),
+ MutableList.of("hello", "world"));
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList("\"hello\", world").get(),
+ MutableList.of("hello", "world"));
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList("[ \"hello\", world ]").get(),
+ MutableList.of("hello", "world"));
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList(" ").get(),
+ MutableList.of());
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList("\"\"").get(),
+ MutableList.of(""));
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList(",,x,").get(),
+ MutableList.of("","","x",""));
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList("\"\",,x,\"\"").get(),
+ MutableList.of("","","x",""));
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList("[ a : 1, b : 2 ]").get(),
+ MutableList.<Object>of(MutableMap.of("a", 1),MutableMap.of("b", 2)));
- Assert.assertEquals(MutableList.<Object>of(1, 2, "buckle my shoe", true, "true", null, "null", ","),
- JavaStringEscapes.tryUnwrapJsonishList("1, 2, buckle my shoe, true, \"true\", null, \"null\", \",\"").get());
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList("1, 2.0, buckle my shoe, true, \"true\", null, \"null\", \",\"").get(),
+ MutableList.<Object>of(1, 2.0, "buckle my shoe", true, "true", null, "null", ","));
try {
JavaStringEscapes.tryUnwrapJsonishList("\"hello").get();
@@ -186,11 +219,13 @@ public class StringEscapesTest {
Asserts.expectedFailureContains(e, "position 1");
}
- Assert.assertEquals(MutableList.of(MutableMap.of("a", "b"), "world"),
- JavaStringEscapes.tryUnwrapJsonishList("[ { a: b }, world ]").get());
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList("[ { a: b }, world ]").get(),
+ MutableList.of(MutableMap.of("a", "b"), "world"));
- Assert.assertEquals(MutableList.of(MutableMap.of("a", MutableList.<Object>of("b", 2)), "world"),
- JavaStringEscapes.tryUnwrapJsonishList("{ a: [ b, 2 ] }, world").get());
+ Assert.assertEquals(
+ JavaStringEscapes.tryUnwrapJsonishList("{ a: [ b, 2 ] }, world").get(),
+ MutableList.of(MutableMap.of("a", MutableList.<Object>of("b", 2)), "world"));
}
}