Repository: ambari Updated Branches: refs/heads/trunk a878047ad -> 1d4cbc8ac (forced update)
AMBARI-22499. Ambari server becomes unusable when config properties are misconfigured (adoroszlai) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/1d4cbc8a Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/1d4cbc8a Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/1d4cbc8a Branch: refs/heads/trunk Commit: 1d4cbc8aca2eddd0294b27755ec5769c66bf6b80 Parents: bce0bd8 Author: Attila Doroszlai <adorosz...@hortonworks.com> Authored: Wed Nov 22 13:30:05 2017 +0100 Committer: Doroszlai, Attila <adorosz...@hortonworks.com> Committed: Thu Nov 23 00:35:09 2017 +0100 ---------------------------------------------------------------------- .../ambari/server/state/ConfigHelper.java | 28 ++++++------ .../ambari/server/state/ConfigHelperTest.java | 47 ++++++++++++++++++++ 2 files changed, 61 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/1d4cbc8a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java index eade914..6813fc0 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java @@ -1503,22 +1503,22 @@ public class ConfigHelper { } /** - * Compares values as double in case they are numbers. - * @param actualValue - * @param newValue - * @return + * Checks for equality of parsed numbers if both values are numeric, + * otherwise using regular equality. */ - private boolean valuesAreEqual(String actualValue, String newValue) { - boolean actualValueIsNumber = NumberUtils.isNumber(actualValue); - boolean newValueIsNumber = NumberUtils.isNumber(newValue); - if (actualValueIsNumber && newValueIsNumber) { - Double ab = Double.parseDouble(actualValue); - Double bb = Double.parseDouble(newValue); - return ab.equals(bb); - } else if (!actualValueIsNumber && !newValueIsNumber) { - return actualValue.equals(newValue); + static boolean valuesAreEqual(String value1, String value2) { // exposed for unit test + if (NumberUtils.isNumber(value1) && NumberUtils.isNumber(value2)) { + try { + Number number1 = NumberUtils.createNumber(value1); + Number number2 = NumberUtils.createNumber(value2); + return Objects.equal(number1, number2) || + number1.doubleValue() == number2.doubleValue(); + } catch (NumberFormatException e) { + // fall back to regular equality + } } - return false; + + return Objects.equal(value1, value2); } /** http://git-wip-us.apache.org/repos/asf/ambari/blob/1d4cbc8a/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java index 8a0a782..ff0bf59 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java @@ -22,6 +22,8 @@ import static org.easymock.EasyMock.createStrictMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import java.sql.SQLException; import java.util.ArrayList; @@ -1212,4 +1214,49 @@ public class ConfigHelperTest { verify(mockAmbariMetaInfo, mockStackVersion, mockServiceInfo, mockPropertyInfo1, mockPropertyInfo2); } } + + public static class RunWithoutModules { + @Test + public void nullsAreEqual() { + assertTrue(ConfigHelper.valuesAreEqual(null, null)); + } + + @Test + public void equalStringsAreEqual() { + assertTrue(ConfigHelper.valuesAreEqual("asdf", "asdf")); + assertTrue(ConfigHelper.valuesAreEqual("qwerty", "qwerty")); + } + + @Test + public void nullIsNotEqualWithNonNull() { + assertFalse(ConfigHelper.valuesAreEqual(null, "asdf")); + assertFalse(ConfigHelper.valuesAreEqual("asdf", null)); + } + + @Test + public void equalNumbersInDifferentFormsAreEqual() { + assertTrue(ConfigHelper.valuesAreEqual("1.234", "1.2340")); + assertTrue(ConfigHelper.valuesAreEqual("12.34", "1.234e1")); + assertTrue(ConfigHelper.valuesAreEqual("123L", "123l")); + assertTrue(ConfigHelper.valuesAreEqual("-1.234", "-1.2340")); + assertTrue(ConfigHelper.valuesAreEqual("-12.34", "-1.234e1")); + assertTrue(ConfigHelper.valuesAreEqual("-123L", "-123l")); + assertTrue(ConfigHelper.valuesAreEqual("1f", "1.0f")); + assertTrue(ConfigHelper.valuesAreEqual("0", "000")); + + // these are treated as different by NumberUtils (due to different types not being equal) + assertTrue(ConfigHelper.valuesAreEqual("123", "123L")); + assertTrue(ConfigHelper.valuesAreEqual("0", "0.0")); + } + + @Test + public void differentNumbersAreNotEqual() { + assertFalse(ConfigHelper.valuesAreEqual("1.234", "1.2341")); + assertFalse(ConfigHelper.valuesAreEqual("123L", "124L")); + assertFalse(ConfigHelper.valuesAreEqual("-1.234", "1.234")); + assertFalse(ConfigHelper.valuesAreEqual("-123L", "123L")); + assertFalse(ConfigHelper.valuesAreEqual("-1.234", "-1.2341")); + assertFalse(ConfigHelper.valuesAreEqual("-123L", "-124L")); + } + } }