Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2583#discussion_r172666072
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1030,8 +1034,35 @@ private static Object normalizeConfValue(Object obj)
{
return ret;
}
- public static boolean isValidConf(Map<String, Object> topoConf) {
- return normalizeConf(topoConf).equals(normalizeConf((Map<String,
Object>) JSONValue.parse(JSONValue.toJSONString(topoConf))));
+ @SuppressWarnings("unchecked")
+ public static boolean isValidConf(Map<String, Object> topoConfIn) {
+ Map<String, Object> origTopoConf = normalizeConf(topoConfIn);
+ Map<String, Object> deserTopoConf = normalizeConf(
+ (Map<String, Object>)
JSONValue.parse(JSONValue.toJSONString(topoConfIn)));
+ return checkMapEquality(origTopoConf, deserTopoConf);
+ }
+
+ @VisibleForTesting
+ static boolean checkMapEquality(Map<String, Object> orig, Map<String,
Object> deser) {
+ MapDifference<String, Object> diff = Maps.difference(orig, deser);
--- End diff --
We need to guard against deser being null.
We are calling `JSONValue.parse` which returns null if it cannot parse an
object. This here will end up throwing an NPE if deser is null. It would
probably be better to switch to `JSONValue.parseWithException` and catch the
exception and handle it instead.
---
