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/8d8bed80 Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/8d8bed80 Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/8d8bed80 Branch: refs/heads/master Commit: 8d8bed80ad2d3edc561214271aa26f1cd87292a5 Parents: 840c6c9 Author: Gergely Hajos <[email protected]> Authored: Sat Mar 3 12:50:55 2018 +0100 Committer: Gergely Hajos <[email protected]> Committed: Sat Mar 3 12:50:55 2018 +0100 ---------------------------------------------------------------------- .../src/jvm/org/apache/storm/utils/Utils.java | 13 ++--- .../jvm/org/apache/storm/utils/UtilsTest.java | 56 ++++++++++++++------ 2 files changed, 48 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/storm/blob/8d8bed80/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 222b6c2..4ba527b 100644 --- a/storm-client/src/jvm/org/apache/storm/utils/Utils.java +++ b/storm-client/src/jvm/org/apache/storm/utils/Utils.java @@ -1039,27 +1039,28 @@ public class Utils { Map<String, Object> origTopoConf = normalizeConf(topoConfIn); Map<String, Object> deserTopoConf = normalizeConf( (Map<String, Object>) JSONValue.parse(JSONValue.toJSONString(topoConfIn))); - return confMapDiff(origTopoConf, deserTopoConf); + return checkMapEquality(origTopoConf, deserTopoConf); } @VisibleForTesting - static boolean confMapDiff(Map<String, Object> orig, Map<String, Object> deser) { + 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()); + 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()); + 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); + entryDiffers.getKey(), leftValue, rightValue); } return false; } http://git-wip-us.apache.org/repos/asf/storm/blob/8d8bed80/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 f595d80..afd8b8d 100644 --- a/storm-client/test/jvm/org/apache/storm/utils/UtilsTest.java +++ b/storm-client/test/jvm/org/apache/storm/utils/UtilsTest.java @@ -178,21 +178,47 @@ 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)); + public void testCheckMapEqualityEmpty() { + Map<String, Object> map0 = ImmutableMap.of(); + Assert.assertTrue(Utils.checkMapEquality(map0, map0)); + } + + @Test + public void testCheckMapEqualityIdentical() { + Map<String, Object> map1 = ImmutableMap.of("k0", ImmutableList.of(1L, 2L), "k1", ImmutableSet.of('s', 'f'), + "k2", "as"); + Assert.assertTrue(Utils.checkMapEquality(map1, map1)); + } + + @Test + public void testCheckMapEqualityEqual() { + Map<String, Object> map1 = ImmutableMap.of("k0", ImmutableList.of(1L, 2L), "k1", ImmutableSet.of('s', 'f'), + "k2", "as"); 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)); + "k2", "as"); + Assert.assertTrue(Utils.checkMapEquality(map1, map2)); // test deep equal + } + + @Test + public void testCheckMapEqualityNotEqual() { + Map<String, Object> map1 = ImmutableMap.of("k0", ImmutableList.of(1L, 2L), "k1", ImmutableSet.of('s', 'f'), + "k2", "as"); + Map<String, Object> map3 = ImmutableMap.of("k0", ImmutableList.of(1L, 2L), "k1", ImmutableSet.of('s', 't'), + "k2", "as"); + Assert.assertFalse(Utils.checkMapEquality(map1, map3)); + } + + @Test + public void testCheckMapEqualityPrimitiveNotEqual() { + Map<String, Object> map4 = ImmutableMap.of("k0", 2L); + Map<String, Object> map5 = ImmutableMap.of("k0", 3L); + Assert.assertFalse(Utils.checkMapEquality(map4, map5)); + } + + @Test + public void testCheckMapEqualityEmptyNotEqual() { + Map<String, Object> map0 = ImmutableMap.of(); + Map<String, Object> map5 = ImmutableMap.of("k0", 3L); + Assert.assertFalse(Utils.checkMapEquality(map0, map5)); } }
