More merging adjustments.

Looks like this should be in pretty good shape and has passed some
of the basic backwards compat. merging tests that I added.

cloudinit/mergers/__init__.py

@@ -25,7 +25,7 @@ from cloudinit import type_utils
 NAME_MTCH = re.compile(r"(^[a-zA-Z_][A-Za-z0-9_]*)\((.*?)\)$")
 
 LOG = logging.getLogger(__name__)
-DEF_MERGE_TYPE = "list()+dict()"
+DEF_MERGE_TYPE = "list()+dict()+str()"
 MERGER_PREFIX = 'm_'
 MERGER_ATTR = 'Merger'
 

cloudinit/mergers/m_dict.py

@@ -16,21 +16,32 @@
 #    You should have received a copy of the GNU General Public License
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+DEF_MERGE_TYPE = 'no_replace'
+MERGE_TYPES = ('replace', DEF_MERGE_TYPE,)
+
+
+def _has_any(what, *keys):
+    for k in keys:
+        if k in what:
+            return True
+    return False
+
 
 class Merger(object):
     def __init__(self, merger, opts):
         self._merger = merger
         # Affects merging behavior...
-        self._method = 'replace'
-        for m in ['replace', 'no_replace']:
+        self._method = DEF_MERGE_TYPE
+        for m in MERGE_TYPES:
             if m in opts:
                 self._method = m
                 break
-        # Affect how recursive merging is done on other primitives
+        # Affect how recursive merging is done on other primitives.
         self._recurse_str = 'recurse_str' in opts
-        self._recurse_dict = True
-        self._recurse_array = 'recurse_array' in opts
+        self._recurse_array = _has_any(opts, 'recurse_array', 'recurse_list')
         self._allow_delete = 'allow_delete' in opts
+        # Backwards compat require this to be on.
+        self._recurse_dict = True
 
     def __str__(self):
         s = ('DictMerger: (method=%s,recurse_str=%s,'
@@ -42,14 +53,14 @@ class Merger(object):
                   self._allow_delete)
         return s
 
-    def _do_dict_replace(self, value, merge_with, do_replace=True):
+    def _do_dict_replace(self, value, merge_with, do_replace):
 
         def merge_same_key(old_v, new_v):
             if do_replace:
                 return new_v
             if isinstance(new_v, (list, tuple)) and self._recurse_array:
                 return self._merger.merge(old_v, new_v)
-            if isinstance(new_v, (str, basestring)) and self._recurse_str:
+            if isinstance(new_v, (basestring)) and self._recurse_str:
                 return self._merger.merge(old_v, new_v)
             if isinstance(new_v, (dict)) and self._recurse_dict:
                 return self._merger.merge(old_v, new_v)
@@ -70,7 +81,7 @@ class Merger(object):
         if not isinstance(merge_with, (dict)):
             return value
         if self._method == 'replace':
-            merged = self._do_dict_replace(dict(value), merge_with)
+            merged = self._do_dict_replace(dict(value), merge_with, True)
         elif self._method == 'no_replace':
             merged = self._do_dict_replace(dict(value), merge_with, False)
         else:

cloudinit/mergers/m_list.py

@@ -32,7 +32,8 @@ class Merger(object):
         self._recurse_array = 'recurse_array' in opts
 
     def __str__(self):
-        return 'ListMerger: (m=%s,rs=%s,rd=%s,ra=%s)' % (self._method,
+        return ('ListMerger: (method=%s,recurse_str=%s,'
+                'recurse_dict=%s,recurse_array=%s)') % (self._method,
                                                         self._recurse_str,
                                                         self._recurse_dict,
                                                         self._recurse_array)

cloudinit/mergers/m_str.py

@@ -0,0 +1,44 @@
+# -*- coding: utf-8 -*-
+# vi: ts=4 expandtab
+#
+# Copyright (C) 2012 Yahoo! Inc.
+#
+# Author: Joshua Harlow <harlowja@yahoo-inc.com>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3, as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+
+class Merger(object):
+    def __init__(self, _merger, opts):
+        self._append = 'append' in opts
+
+    def __str__(self):
+        return 'StringMerger: (append=%s)' % (self._append)
+
+    # On encountering a unicode object to merge value with
+    # we will for now just proxy into the string method to let it handle it.
+    def _on_unicode(self, value, merge_with):
+        return self._on_str(value, merge_with)
+
+    # On encountering a string object to merge with we will
+    # perform the following action, if appending we will
+    # merge them together, otherwise we will just return value.
+    def _on_str(self, value, merge_with):
+        if not isinstance(value, (basestring)):
+            return merge_with
+        if not self._append:
+            return merge_with
+        if isinstance(value, unicode):
+            return value + unicode(merge_with)
+        else:
+            return value + str(merge_with)

tests/data/merge_sources/expected2.yaml

@@ -1,3 +1,3 @@
-Blah: 3
+Blah: 1
 Blah2: 2
-Blah3: [1]
+Blah3: 3

tests/data/merge_sources/expected5.yaml

@@ -0,0 +1,7 @@
+#cloud-config
+
+Blah: 3
+Blah2: 2
+Blah3: [1]
+
+

tests/data/merge_sources/source5-1.yaml

@@ -0,0 +1,6 @@
+#cloud-config
+
+
+Blah: 1
+Blah2: 2
+Blah3: 3

tests/data/merge_sources/source5-2.yaml

@@ -0,0 +1,8 @@
+#cloud-config
+
+Blah: 3
+Blah2: 2
+Blah3: [1]
+
+
+merge_how: 'dict(replace)+list(append)'

tests/unittests/test_merging.py

@@ -11,14 +11,39 @@ import glob
 import os
 import re
 
+SOURCE_PAT = "source*.*yaml"
+EXPECTED_PAT = "expected%s.yaml" 
+
+
+def _old_mergedict(src, cand):
+    """
+    Merge values from C{cand} into C{src}.
+    If C{src} has a key C{cand} will not override.
+    Nested dictionaries are merged recursively.
+    """
+    if isinstance(src, dict) and isinstance(cand, dict):
+        for (k, v) in cand.iteritems():
+            if k not in src:
+                src[k] = v
+            else:
+                src[k] = _old_mergedict(src[k], v)
+    return src
+
+
+def _old_mergemanydict(*args):
+    out = {}
+    for a in args:
+        out = _old_mergedict(out, a)
+    return out
+
 
 class TestSimpleRun(helpers.ResourceUsingTestCase):
-    def _load_merge_files(self, data_dir):
-        merge_root = self.resourceLocation(data_dir)
+    def _load_merge_files(self):
+        merge_root = self.resourceLocation('merge_sources')
         tests = []
         source_ids = collections.defaultdict(list)
         expected_files = {}
-        for fn in glob.glob(os.path.join(merge_root, "source*.*yaml")):
+        for fn in glob.glob(os.path.join(merge_root, SOURCE_PAT)):
             base_fn = os.path.basename(fn)
             file_id = re.match(r"source(\d+)\-(\d+)[.]yaml", base_fn)
             if not file_id:
@@ -26,31 +51,97 @@ class TestSimpleRun(helpers.ResourceUsingTestCase):
                               % (fn))
             file_id = int(file_id.group(1))
             source_ids[file_id].append(fn)
-            expected_fn = os.path.join(merge_root,
-                                       "expected%s.yaml" % (file_id))
+            expected_fn = os.path.join(merge_root, EXPECTED_PAT % (file_id))
             if not os.path.isfile(expected_fn):
                 raise IOError("No expected file found at %s" % (expected_fn))
             expected_files[file_id] = expected_fn
-        for id in sorted(source_ids.keys()):
+        for i in sorted(source_ids.keys()):
             source_file_contents = []
-            for fn in sorted(source_ids[id]):
-                source_file_contents.append(util.load_file(fn))
-            expected = util.load_yaml(util.load_file(expected_files[id]))
-            tests.append((source_file_contents, expected))
+            for fn in sorted(source_ids[i]):
+                source_file_contents.append([fn, util.load_file(fn)])
+            expected = util.load_yaml(util.load_file(expected_files[i]))
+            entry = [source_file_contents, [expected, expected_files[i]]]
+            tests.append(entry)
         return tests
 
     def test_merge_samples(self):
-        tests = self._load_merge_files('merge_sources')
+        tests = self._load_merge_files()
         paths = c_helpers.Paths({})
         cc_handler = cloud_config.CloudConfigPartHandler(paths)
         cc_handler.cloud_fn = None
-        for (payloads, expected_merge) in tests:
+        for (payloads, (expected_merge, expected_fn)) in tests:
             cc_handler.handle_part(None, CONTENT_START, None,
                                    None, None, None)
-            for (i, p) in enumerate(payloads):
-                cc_handler.handle_part(None, None, "t-%s.yaml" % (i + 1),
-                                       p, None, {})
+            merging_fns = []
+            for (fn, contents) in payloads:
+                cc_handler.handle_part(None, None, "%s.yaml" % (fn),
+                                       contents, None, {})
+                merging_fns.append(fn)
             merged_buf = cc_handler.cloud_buf
             cc_handler.handle_part(None, CONTENT_END, None,
                                    None, None, None)
-            self.assertEquals(expected_merge, merged_buf)
+            fail_msg = "Equality failure on checking %s with %s: %s != %s"
+            fail_msg = fail_msg % (expected_fn,
+                                   ",".join(merging_fns), merged_buf,
+                                   expected_merge)
+            self.assertEquals(expected_merge, merged_buf, msg=fail_msg)
+
+    def test_compat_merges_dict(self):
+        a = {
+            '1': '2',
+            'b': 'c',
+        }
+        b = {
+            'b': 'e',
+        }
+        c = _old_mergedict(a, b)
+        d = util.mergemanydict([a, b])
+        self.assertEquals(c, d)
+
+    def test_compat_merges_list(self):
+        a = {'b': [1, 2, 3]}
+        b = {'b': [4, 5]}
+        c = {'b': [6, 7]}
+        e = _old_mergemanydict(a, b, c)
+        f = util.mergemanydict([a, b, c])
+        self.assertEquals(e, f)
+
+    def test_compat_merges_str(self):
+        a = {'b': "hi"}
+        b = {'b': "howdy"}
+        c = {'b': "hallo"}
+        e = _old_mergemanydict(a, b, c)
+        f = util.mergemanydict([a, b, c])
+        self.assertEquals(e, f)
+
+    def test_compat_merge_sub_dict(self):
+        a = {
+            '1': '2',
+            'b': {
+                'f': 'g',
+            }
+        }
+        b = {
+            'b': {
+                'e': 'c',
+            }
+        }
+        c = _old_mergedict(a, b)
+        d = util.mergemanydict([a, b])
+        self.assertEquals(c, d)
+
+    def test_compat_merge_sub_list(self):
+        a = {
+            '1': '2',
+            'b': {
+                'f': ['1'],
+            }
+        }
+        b = {
+            'b': {
+                'f': [],
+            }
+        }
+        c = _old_mergedict(a, b)
+        d = util.mergemanydict([a, b])
+        self.assertEquals(c, d)

tests/unittests/test_userdata.py

@@ -60,7 +60,6 @@ run:
  - c
 '''
         message1 = MIMEBase("text", "cloud-config")
-        message1['Merge-Type'] = 'dict()+list(extend)+str(append)'
         message1.set_payload(blob)
 
         blob2 = '''
@@ -72,7 +71,8 @@ run:
  - morestuff
 '''
         message2 = MIMEBase("text", "cloud-config")
-        message2['X-Merge-Type'] = 'dict()+list(extend)+str()'
+        message2['X-Merge-Type'] = ('dict(recurse_array,'
+                                    'recurse_str)+list(append)+str(append)')
         message2.set_payload(blob2)
 
         blob3 = '''
@@ -84,7 +84,6 @@ e:
 p: 1
 '''
         message3 = MIMEBase("text", "cloud-config")
-        message3['Merge-Type'] = 'dict()+list()+str()'
         message3.set_payload(blob3)
 
         messages = [message1, message2, message3]