Fix delete notation merge where no variables are used (#467)

* if delete notation affects previous model, just delete key from dict during merge

* update readme for change in functionality

Author: ddsharpe

README.md

@@ -305,25 +305,29 @@ If variable properties are used in element names, such as ```@@PROP:my-server@@`
 
 #### Multiple Models and Delete Notation
 
-A named element using [delete notation](#declaring-named-mbeans-to-delete) will completely replace an element with a matching name and no delete notation in a previous model. For example, if Model 1 looks like:
+A named element using [delete notation](#declaring-named-mbeans-to-delete) will completely delete an element with a matching name and no delete notation in a previous model. For example, if Model 1 looks like:
 ```yaml
 topology:
     Server:
         m1:
             ListenPort: 7000 
             Notes: "Server 1"
+        m2:
+            ListenPort: 9000
 ```
 and Model 2 looks like:
 ```yaml
 topology:
     Server:
-        !m1:
+        !m2:
 ```
 The resulting model would be:
 ```yaml
 topology:
     Server:
-        !m1:
+        m1:
+            ListenPort: 7000 
+            Notes: "Server 1"
 ```
 
 Similarly, an element without delete notation will completely replace an element with a matching name that has delete notation in a previous model. For example, if Model 1 looks like:

core/src/main/python/deploy.py

@@ -45,7 +45,6 @@
 from wlsdeploy.util.cla_utils import CommandLineArgUtil
 from wlsdeploy.util.model import Model
 from wlsdeploy.util.model_context import ModelContext
-from wlsdeploy.util.model_translator import FileToPython
 from wlsdeploy.util.weblogic_helper import WebLogicHelper
 
 wlst_extended.wlst_functions = globals()

core/src/main/python/wlsdeploy/util/cla_helper.py

@@ -150,7 +150,8 @@ def _merge_dictionaries(dictionary, new_dictionary, variable_map):
         # the new key should replace the existing one - delete the existing key and add the new one
         elif replace_key:
             del dictionary[dictionary_key]
-            dictionary[new_key] = new_value
+            if not deployer_utils.is_delete_name(new_key):
+                dictionary[new_key] = new_value
 
         # the key is in both dictionaries - merge if the values are dictionaries, otherwise replace the value
         else:
@@ -175,16 +176,15 @@ def _find_dictionary_merge_key(dictionary, new_key, variable_map):
     if new_key in dictionary:
         return new_key, False
 
-    if variable_map is not None:
-        new_is_delete = deployer_utils.is_delete_name(new_key)
-        match_new_key = _get_merge_match_key(new_key, variable_map)
+    new_is_delete = deployer_utils.is_delete_name(new_key)
+    match_new_key = _get_merge_match_key(new_key, variable_map)
 
-        for dictionary_key in dictionary.keys():
-            dictionary_is_delete = deployer_utils.is_delete_name(dictionary_key)
-            match_dictionary_key = _get_merge_match_key(dictionary_key, variable_map)
-            if match_dictionary_key == match_new_key:
-                replace_key = new_is_delete != dictionary_is_delete
-                return dictionary_key, replace_key
+    for dictionary_key in dictionary.keys():
+        dictionary_is_delete = deployer_utils.is_delete_name(dictionary_key)
+        match_dictionary_key = _get_merge_match_key(dictionary_key, variable_map)
+        if match_dictionary_key == match_new_key:
+            replace_key = new_is_delete != dictionary_is_delete
+            return dictionary_key, replace_key
 
     return None, False
 
@@ -197,7 +197,11 @@ def _get_merge_match_key(key, variable_map):
     :param variable_map: variable map to use for substitutions
     :return: the key to use for matching
     """
-    match_key = variables.substitute_key(key, variable_map)
+    if variable_map is not None:
+        match_key = variables.substitute_key(key, variable_map)
+    else:
+        match_key = key
+
     if deployer_utils.is_delete_name(match_key):
         match_key = deployer_utils.get_delete_item_name(match_key)
     return match_key

core/src/test/python/wlsdeploy/util/cla_helper_test.py

@@ -99,7 +99,7 @@ def testMergeNameToProperty(self):
 
         self._check_merged_server(dictionary, '@@PROP:server1a@@')
 
-    # an element with delete notation should replace a base element with a matching name and no notation.
+    # an element with delete notation should remove a base element with a matching name.
     def testMergeDeletedNameToName(self):
         dictionary = _build_model_one('m1')
         new_dictionary = _build_delete_model('m1')
@@ -108,8 +108,20 @@ def testMergeDeletedNameToName(self):
         cla_helper._merge_dictionaries(dictionary, new_dictionary, variables)
         # print("Merged model: " + str(dictionary))
 
+        servers = dictionary['Servers']
+        self.assertEquals(0, len(servers), "there should be no servers after delete")
+
+    # an element with delete notation should leave a base element with a matching delete notation.
+    def testMergeDeletedNameToDeleteName(self):
+        dictionary = _build_delete_model('m1')
+        new_dictionary = _build_delete_model('m1')
+        variables = {}
+
+        cla_helper._merge_dictionaries(dictionary, new_dictionary, variables)
+        # print("Merged model: " + str(dictionary))
+
         server = self._check_single_server(dictionary, '!m1')
-        self.assertEquals(0, len(server), "delete server should have no attributes")
+        self.assertEquals(0, len(server), "server should have no attributes")
 
     # an element should replace a base element with a matching name and delete notation.
     def testMergeNameToDeletedName(self):