Repository: storm Updated Branches: refs/heads/master 76af86b58 -> 402a371cc
STORM-2649 More detailed check of config serialization Project: http://git-wip-us.apache.org/repos/asf/storm/repo Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/840c6c96 Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/840c6c96 Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/840c6c96 Branch: refs/heads/master Commit: 840c6c969f0fda0292c4fe3c07a3126ab1072d18 Parents: a38f0c5 Author: Gergely Hajos <[email protected]> Authored: Sun Feb 25 19:32:45 2018 +0100 Committer: Gergely Hajos <[email protected]> Committed: Fri Mar 2 15:42:50 2018 +0100 ---------------------------------------------------------------------- .../src/jvm/org/apache/storm/utils/Utils.java | 34 ++++++++++++++++++-- .../jvm/org/apache/storm/utils/UtilsTest.java | 24 +++++++++++++- 2 files changed, 55 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/storm/blob/840c6c96/storm-client/src/jvm/org/apache/storm/utils/Utils.java ---------------------------------------------------------------------- diff --git a/storm-client/src/jvm/org/apache/storm/utils/Utils.java b/storm-client/src/jvm/org/apache/storm/utils/Utils.java index 26d267e..222b6c2 100644 --- a/storm-client/src/jvm/org/apache/storm/utils/Utils.java +++ b/storm-client/src/jvm/org/apache/storm/utils/Utils.java @@ -65,6 +65,10 @@ import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; import com.google.common.collect.Lists; +import com.google.common.collect.MapDifference; +import com.google.common.collect.MapDifference.ValueDifference; +import com.google.common.collect.Maps; + import org.apache.commons.io.FileUtils; import org.apache.commons.io.input.ClassLoaderObjectInputStream; import org.apache.storm.Config; @@ -1030,8 +1034,34 @@ public class Utils { 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 confMapDiff(origTopoConf, deserTopoConf); + } + + @VisibleForTesting + static boolean confMapDiff(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()) { + LOG.warn("Some config property changed during serialization. Changed Name: {} - Value: {}", entryOnRight.getKey(), + entryOnRight.getValue()); + } + for (Map.Entry<String, ValueDifference<Object>> entryDiffers : diff.entriesDiffering().entrySet()) { + Object leftValue = entryDiffers.getValue().leftValue(); + Object rightValue = entryDiffers.getValue().rightValue(); + LOG.warn("Config value differs after json serialization. Name: {} - Original Value: {} - DeSer. Value: {}", + entryDiffers.getKey(), leftValue, rightValue); + } + return false; } public static TopologyInfo getTopologyInfo(String name, String asUser, Map<String, Object> topoConf) { http://git-wip-us.apache.org/repos/asf/storm/blob/840c6c96/storm-client/test/jvm/org/apache/storm/utils/UtilsTest.java ---------------------------------------------------------------------- diff --git a/storm-client/test/jvm/org/apache/storm/utils/UtilsTest.java b/storm-client/test/jvm/org/apache/storm/utils/UtilsTest.java index c8499e9..f595d80 100644 --- a/storm-client/test/jvm/org/apache/storm/utils/UtilsTest.java +++ b/storm-client/test/jvm/org/apache/storm/utils/UtilsTest.java @@ -18,16 +18,19 @@ package org.apache.storm.utils; +import org.apache.curator.shaded.com.google.common.collect.ImmutableList; +import org.apache.curator.shaded.com.google.common.collect.ImmutableSet; import org.apache.storm.Config; import org.apache.thrift.transport.TTransportException; import org.junit.Assert; import org.junit.Test; +import com.google.common.collect.ImmutableMap; + import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.ArrayList; public class UtilsTest { @@ -173,4 +176,23 @@ public class UtilsTest { } } } + + @Test + public void testMapDiff() { + Map<String, Object> map0 = ImmutableMap.of(); + Assert.assertTrue("case0", Utils.confMapDiff(map0, map0)); + Map<String, Object> map1 = ImmutableMap.of("k0", ImmutableList.of(1L, 2L), "k1", ImmutableSet.of('s', 'f'), + "k2", "as"); + Assert.assertTrue("case1", Utils.confMapDiff(map1, map1)); + Map<String, Object> map2 = ImmutableMap.of("k0", ImmutableList.of(1L, 2L), "k1", ImmutableSet.of('s', 'f'), + "k2", "as"); + Assert.assertTrue("case2", Utils.confMapDiff(map2, map2)); + Map<String, Object> map3 = ImmutableMap.of("k0", ImmutableList.of(1L, 2L), "k1", ImmutableSet.of('s', 't'), + "k2", "as"); + Assert.assertFalse("case3", Utils.confMapDiff(map1, map3)); + Map<String, Object> map4 = ImmutableMap.of("k0", 2L); + Map<String, Object> map5 = ImmutableMap.of("k0", 3L); + Assert.assertFalse("case4", Utils.confMapDiff(map4, map5)); + Assert.assertFalse("case5", Utils.confMapDiff(map0, map5)); + } }
