ambari-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jspei...@apache.org
Subject ambari git commit: AMBARI-12366. Fix issue where the BP configuration is modified on a getter invocation due to an incomplete deep copy
Date Thu, 09 Jul 2015 23:07:08 GMT
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 <jspeidel@hortonworks.com>
Authored: Thu Jul 9 17:45:13 2015 -0400
Committer: John Speidel <jspeidel@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());
 


Mime
View raw message