You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by js...@apache.org on 2015/07/10 01:07:08 UTC
ambari git commit: AMBARI-12366. Fix issue where the BP configuration
is modified on a getter invocation due to an incomplete deep copy
Repository: ambari
Updated Branches:
refs/heads/trunk 630299fb3 -> ba21f8988
AMBARI-12366. Fix issue where the BP configuration is modified on a getter invocation
due to an incomplete deep copy
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/ba21f898
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/ba21f898
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/ba21f898
Branch: refs/heads/trunk
Commit: ba21f89881d80de8be1256336fa6f655db02fd54
Parents: 630299f
Author: John Speidel <js...@hortonworks.com>
Authored: Thu Jul 9 17:45:13 2015 -0400
Committer: John Speidel <js...@hortonworks.com>
Committed: Thu Jul 9 19:06:45 2015 -0400
----------------------------------------------------------------------
.../ambari/server/topology/Configuration.java | 12 +++++--
.../server/topology/ConfigurationTest.java | 37 +++++++-------------
2 files changed, 21 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/ambari/blob/ba21f898/ambari-server/src/main/java/org/apache/ambari/server/topology/Configuration.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/Configuration.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/Configuration.java
index 91b7736..108ff74 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/Configuration.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/Configuration.java
@@ -115,7 +115,11 @@ public class Configuration {
*/
public Map<String, Map<String, String>> getFullProperties(int depthLimit) {
if (depthLimit == 0) {
- return new HashMap<String, Map<String, String>>(properties);
+ HashMap<String, Map<String, String>> propertiesCopy = new HashMap<String, Map<String, String>>();
+ for (Map.Entry<String, Map<String, String>> typeProperties : properties.entrySet()) {
+ propertiesCopy.put(typeProperties.getKey(), new HashMap<String, String>(typeProperties.getValue()));
+ }
+ return propertiesCopy;
}
Map<String, Map<String, String>> mergedProperties = parentConfiguration == null ?
@@ -159,8 +163,10 @@ public class Configuration {
for (Map.Entry<String, Map<String, Map<String, String>>> typeEntry : attributes.entrySet()) {
String type = typeEntry.getKey();
- Map<String, Map<String, String>> typeAttributes =
- new HashMap<String, Map<String, String>>(typeEntry.getValue());
+ Map<String, Map<String, String>> typeAttributes = new HashMap<String, Map<String, String>>();
+ for (Map.Entry<String, Map<String, String>> attributeEntry : typeEntry.getValue().entrySet()) {
+ typeAttributes.put(attributeEntry.getKey(), new HashMap<String, String>(attributeEntry.getValue()));
+ }
if (! mergedAttributeMap.containsKey(type)) {
mergedAttributeMap.put(type, typeAttributes);
http://git-wip-us.apache.org/repos/asf/ambari/blob/ba21f898/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurationTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurationTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurationTest.java
index 9bab88f..e971e03 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurationTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurationTest.java
@@ -106,12 +106,6 @@ public class ConfigurationTest {
@Test
public void testGetFullProperties_withParent() {
Configuration configuration = createConfigurationWithParents_PropsOnly();
- // get prop maps prior to calling getFullProperties
- Map<String, Map<String, String>> leafProperties = configuration.getProperties();
- Map<String, Map<String, String>> parentProperties = configuration.getParentConfiguration().getProperties();
- Map<String, Map<String, String>> parentParentProperties = configuration.getParentConfiguration().getParentConfiguration().getProperties();
-
- // test
// all parents should be reflected in getFullProperties() result
Map<String, Map<String, String>> fullProperties = configuration.getFullProperties();
@@ -145,9 +139,10 @@ public class ConfigurationTest {
assertEquals("val11.3", type4Props.get("prop11"));
// ensure that underlying property map is not modified in getFullProperties
- assertEquals(leafProperties, configuration.getProperties());
- assertEquals(parentProperties, configuration.getParentConfiguration().getProperties());
- assertEquals(parentParentProperties, configuration.getParentConfiguration().getParentConfiguration().getProperties());
+ Configuration expectedConfiguration = createConfigurationWithParents_PropsOnly();
+ assertEquals(expectedConfiguration.getProperties(), configuration.getProperties());
+ assertEquals(expectedConfiguration.getParentConfiguration().getProperties(), configuration.getParentConfiguration().getProperties());
+ assertEquals(expectedConfiguration.getParentConfiguration().getParentConfiguration().getProperties(), configuration.getParentConfiguration().getParentConfiguration().getProperties());
assertEquals(EMPTY_ATTRIBUTES, configuration.getAttributes());
@@ -159,12 +154,6 @@ public class ConfigurationTest {
@Test
public void testGetFullProperties_withParent_specifyDepth() {
Configuration configuration = createConfigurationWithParents_PropsOnly();
- // get prop maps prior to calling getFullProperties
- Map<String, Map<String, String>> leafProperties = configuration.getProperties();
- Map<String, Map<String, String>> parentProperties = configuration.getParentConfiguration().getProperties();
- Map<String, Map<String, String>> parentParentProperties = configuration.getParentConfiguration().getParentConfiguration().getProperties();
-
- // test
// specify a depth of 1 which means to include only 1 level up the parent chain
Map<String, Map<String, String>> fullProperties = configuration.getFullProperties(1);
@@ -196,9 +185,10 @@ public class ConfigurationTest {
assertEquals("val11.3", type4Props.get("prop11"));
// ensure that underlying property maps are not modified in getFullProperties
- assertEquals(leafProperties, configuration.getProperties());
- assertEquals(parentProperties, configuration.getParentConfiguration().getProperties());
- assertEquals(parentParentProperties, configuration.getParentConfiguration().getParentConfiguration().getProperties());
+ Configuration expectedConfiguration = createConfigurationWithParents_PropsOnly();
+ assertEquals(expectedConfiguration.getProperties(), configuration.getProperties());
+ assertEquals(expectedConfiguration.getParentConfiguration().getProperties(), configuration.getParentConfiguration().getProperties());
+ assertEquals(expectedConfiguration.getParentConfiguration().getParentConfiguration().getProperties(), configuration.getParentConfiguration().getParentConfiguration().getProperties());
assertEquals(EMPTY_ATTRIBUTES, configuration.getAttributes());
}
@@ -228,10 +218,6 @@ public class ConfigurationTest {
@Test
public void testGetFullAttributes_withParent() {
Configuration configuration = createConfigurationWithParents_AttributesOnly();
- Map<String, Map<String, Map<String, String>>> leafAttributes = configuration.getAttributes();
- Map<String, Map<String, Map<String, String>>> parentAttributes = configuration.getParentConfiguration().getAttributes();
- Map<String, Map<String, Map<String, String>>> parentParentAttributes = configuration.getParentConfiguration().getParentConfiguration().getAttributes();
- // test
// all parents should be reflected in getFullAttributes() result
Map<String, Map<String, Map<String, String>>> fullAttributes = configuration.getFullAttributes();
assertEquals(2, fullAttributes.size());
@@ -284,9 +270,10 @@ public class ConfigurationTest {
assertEquals("val101.1", attribute101Properties.get("prop101"));
// ensure that underlying attribute maps are not modified in getFullProperties
- assertEquals(leafAttributes, configuration.getAttributes());
- assertEquals(parentAttributes, configuration.getParentConfiguration().getAttributes());
- assertEquals(parentParentAttributes, configuration.getParentConfiguration().getParentConfiguration().getAttributes());
+ Configuration expectedConfiguration = createConfigurationWithParents_AttributesOnly();
+ assertEquals(expectedConfiguration.getAttributes(), configuration.getAttributes());
+ assertEquals(expectedConfiguration.getParentConfiguration().getAttributes(), configuration.getParentConfiguration().getAttributes());
+ assertEquals(expectedConfiguration.getParentConfiguration().getParentConfiguration().getAttributes(), configuration.getParentConfiguration().getParentConfiguration().getAttributes());
assertEquals(EMPTY_PROPERTIES, configuration.getProperties());