Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2583#discussion_r172666722
--- 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);
+ if (diff.areEqual()) {
+ return true;
+ }
+ for (Map.Entry<String, Object> entryOnLeft :
diff.entriesOnlyOnLeft().entrySet()) {
+ LOG.warn("Config property not serializable. Name: {} - Value: {}",
entryOnLeft.getKey(),
+ entryOnLeft.getValue());
+ }
+ for (Map.Entry<String, Object> entryOnRight :
diff.entriesOnlyOnRight().entrySet()) {
--- End diff --
I don't know of any situation where a key would be in one map but not the
other map with the current JSON serialization/parsing. I am fine with leaving
these checks in for completeness, but it might be good to change the warning
messages to explain to the end user that something really odd happened with key
"X", but because this should never happen in real life you might be on your own
with debugging it.
---
