AMBARI-6759. Switching host to config-group with final flag turned off does not reflect in file

ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java

@@ -225,11 +225,7 @@ public class ConfigHelper {
         for (Entry<String, String> overrideEntry : tags.entrySet()) {
           Config overrideConfig = cluster.getConfig(type,
               overrideEntry.getValue());
-
-          // TODO clarify correct behavior for attributes overriding
-          if (overrideConfig != null) {
-            cloneAttributesMap(overrideConfig.getPropertiesAttributes(), attributesMap);
-          }
+          overrideAttributes(overrideConfig, attributesMap);
         }
         if (attributesMap != null) {
           attributes.put(type, attributesMap);
@@ -270,6 +266,36 @@ public class ConfigHelper {
     return finalConfig;
   }
 
+  /**
+   * Merge override attributes with original ones.
+   * If overrideConfig#getPropertiesAttributes does not contain occurrence of override for any of
+   * properties from overrideConfig#getProperties then persisted attribute should be removed.
+   */
+  public Map<String, Map<String, String>> overrideAttributes(Config overrideConfig,
+                                                             Map<String, Map<String, String>> persistedAttributes) {
+    if (overrideConfig != null && persistedAttributes != null) {
+      Map<String, Map<String, String>> overrideAttributes = overrideConfig.getPropertiesAttributes();
+      if (overrideAttributes != null) {
+        cloneAttributesMap(overrideAttributes, persistedAttributes);
+        Map<String, String> overrideProperties = overrideConfig.getProperties();
+        if (overrideProperties != null) {
+          Set<String> overriddenProperties = overrideProperties.keySet();
+          for (String overriddenProperty : overriddenProperties) {
+            for (Entry<String, Map<String, String>> persistedAttribute : persistedAttributes.entrySet()) {
+              String attributeName = persistedAttribute.getKey();
+              Map<String, String> persistedAttributeValues = persistedAttribute.getValue();
+              Map<String, String> overrideAttributeValues = overrideAttributes.get(attributeName);
+              if (overrideAttributeValues == null || !overrideAttributeValues.containsKey(overriddenProperty)) {
+                persistedAttributeValues.remove(overriddenProperty);
+              }
+            }
+          }
+        }
+      }
+    }
+    return persistedAttributes;
+  }
+
   public void cloneAttributesMap(Map<String, Map<String, String>> sourceAttributesMap,
                                  Map<String, Map<String, String>> targetAttributesMap) {
     if (sourceAttributesMap != null && targetAttributesMap != null) {

ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java

@@ -40,6 +40,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 public class ConfigHelperTest {
   private Clusters clusters;
@@ -378,4 +379,119 @@ public class ConfigHelperTest {
     Assert.assertEquals("7", attributes.get("f"));
     Assert.assertEquals("8", attributes.get("q"));
   }
+
+  @Test
+  public void testMergeAttributes() throws Exception {
+    Map<String, Map<String, String>> persistedAttributes = new HashMap<String, Map<String, String>>();
+    Map<String, String> persistedFinalAttrs = new HashMap<String, String>();
+    persistedFinalAttrs.put("a", "true");
+    persistedFinalAttrs.put("c", "true");
+    persistedFinalAttrs.put("d", "true");
+    persistedAttributes.put("final", persistedFinalAttrs);
+    Map<String, Map<String, String>> confGroupAttributes = new HashMap<String, Map<String, String>>();
+    Map<String, String> confGroupFinalAttrs = new HashMap<String, String>();
+    confGroupFinalAttrs.put("b", "true");
+    confGroupAttributes.put("final", confGroupFinalAttrs);
+    Map<String, String> confGroupProperties = new HashMap<String, String>();
+    confGroupProperties.put("a", "any");
+    confGroupProperties.put("b", "any");
+    confGroupProperties.put("c", "any");
+
+    Config overrideConfig = new ConfigImpl(cluster, "type", confGroupProperties, confGroupAttributes, injector);
+
+    Map<String, Map<String, String>> result
+        = configHelper.overrideAttributes(overrideConfig, persistedAttributes);
+
+    Assert.assertNotNull(result);
+    Assert.assertEquals(1, result.size());
+    Map<String, String> finalResultAttributes = result.get("final");
+    Assert.assertNotNull(finalResultAttributes);
+    Assert.assertEquals(2, finalResultAttributes.size());
+    Assert.assertEquals("true", finalResultAttributes.get("b"));
+    Assert.assertEquals("true", finalResultAttributes.get("d"));
+  }
+
+  @Test
+  public void testMergeAttributes_noAttributeOverrides() throws Exception {
+    Map<String, Map<String, String>> persistedAttributes = new HashMap<String, Map<String, String>>();
+    Map<String, String> persistedFinalAttrs = new HashMap<String, String>();
+    persistedFinalAttrs.put("a", "true");
+    persistedFinalAttrs.put("c", "true");
+    persistedFinalAttrs.put("d", "true");
+    persistedAttributes.put("final", persistedFinalAttrs);
+    Map<String, Map<String, String>> confGroupAttributes = new HashMap<String, Map<String, String>>();
+    Map<String, String> confGroupProperties = new HashMap<String, String>();
+    confGroupProperties.put("a", "any");
+    confGroupProperties.put("b", "any");
+    confGroupProperties.put("c", "any");
+
+    Config overrideConfig = new ConfigImpl(cluster, "type", confGroupProperties, confGroupAttributes, injector);
+
+    Map<String, Map<String, String>> result
+        = configHelper.overrideAttributes(overrideConfig, persistedAttributes);
+
+    Assert.assertNotNull(result);
+    Assert.assertEquals(1, result.size());
+    Map<String, String> finalResultAttributes = result.get("final");
+    Assert.assertNotNull(finalResultAttributes);
+    Assert.assertEquals(1, finalResultAttributes.size());
+    Assert.assertEquals("true", finalResultAttributes.get("d"));
+  }
+
+  @Test
+  public void testMergeAttributes_nullAttributes() throws Exception {
+    Map<String, Map<String, String>> persistedAttributes = new HashMap<String, Map<String, String>>();
+    Map<String, String> persistedFinalAttrs = new HashMap<String, String>();
+    persistedFinalAttrs.put("a", "true");
+    persistedFinalAttrs.put("c", "true");
+    persistedFinalAttrs.put("d", "true");
+    persistedAttributes.put("final", persistedFinalAttrs);
+    Map<String, String> confGroupProperties = new HashMap<String, String>();
+    confGroupProperties.put("a", "any");
+    confGroupProperties.put("b", "any");
+    confGroupProperties.put("c", "any");
+
+    Config overrideConfig = new ConfigImpl(cluster, "type", confGroupProperties, null, injector);
+
+    Map<String, Map<String, String>> result
+        = configHelper.overrideAttributes(overrideConfig, persistedAttributes);
+
+    Assert.assertNotNull(result);
+    Assert.assertEquals(1, result.size());
+    Map<String, String> finalResultAttributes = result.get("final");
+    Assert.assertNotNull(finalResultAttributes);
+    Assert.assertEquals(3, finalResultAttributes.size());
+    Assert.assertEquals("true", finalResultAttributes.get("a"));
+    Assert.assertEquals("true", finalResultAttributes.get("c"));
+    Assert.assertEquals("true", finalResultAttributes.get("d"));
+  }
+
+  @Test
+  public void testMergeAttributes_nullProperties() throws Exception {
+    Map<String, Map<String, String>> persistedAttributes = new HashMap<String, Map<String, String>>();
+    Map<String, String> persistedFinalAttrs = new HashMap<String, String>();
+    persistedFinalAttrs.put("a", "true");
+    persistedFinalAttrs.put("c", "true");
+    persistedFinalAttrs.put("d", "true");
+    persistedAttributes.put("final", persistedFinalAttrs);
+    Map<String, Map<String, String>> confGroupAttributes = new HashMap<String, Map<String, String>>();
+    Map<String, String> confGroupFinalAttrs = new HashMap<String, String>();
+    confGroupFinalAttrs.put("b", "true");
+    confGroupAttributes.put("final", confGroupFinalAttrs);
+
+    Config overrideConfig = new ConfigImpl(cluster, "type", null, confGroupAttributes, injector);
+
+    Map<String, Map<String, String>> result
+        = configHelper.overrideAttributes(overrideConfig, persistedAttributes);
+
+    Assert.assertNotNull(result);
+    Assert.assertEquals(1, result.size());
+    Map<String, String> finalResultAttributes = result.get("final");
+    Assert.assertNotNull(finalResultAttributes);
+    Assert.assertEquals(4, finalResultAttributes.size());
+    Assert.assertEquals("true", finalResultAttributes.get("a"));
+    Assert.assertEquals("true", finalResultAttributes.get("b"));
+    Assert.assertEquals("true", finalResultAttributes.get("c"));
+    Assert.assertEquals("true", finalResultAttributes.get("d"));
+  }
 }