[3374] Reverted the changes made in 3339 and 3373

Files reverted:

src/lib/cc/data.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2010-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -964,26 +964,9 @@ merge(ElementPtr element, ConstElementPtr other) {
 
     const std::map<std::string, ConstElementPtr>& m = other->mapValue();
     for (std::map<std::string, ConstElementPtr>::const_iterator it = m.begin();
-        it != m.end() ; ++it) {
+         it != m.end() ; ++it) {
         if ((*it).second && (*it).second->getType() != Element::null) {
-            if (((*it).second->getType() == Element::map) &&
-                element->contains((*it).first)) {
-                // Sub-element is a map and is also in the original config,
-                // so we need to merge them too.
-                boost::shared_ptr<MapElement> merged_map(new MapElement());
-                ConstElementPtr orig_map = element->get((*it).first);
-                ConstElementPtr other_map = (*it).second;
-                if (orig_map->getType() ==  Element::map) {
-                    merged_map->setValue(orig_map->mapValue());
-                }
-
-                // Now go recursive to merge the map sub-elements.
-                merge(merged_map, other_map);
-                element->set((*it).first, merged_map);
-            }
-            else {
-                element->set((*it).first, (*it).second);
-            }
+            element->set((*it).first, (*it).second);
         } else if (element->contains((*it).first)) {
             element->remove((*it).first);
         }

src/lib/cc/tests/data_unittests.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2009-2014  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2009  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -848,7 +848,7 @@ TEST(Element, merge) {
     ElementPtr a = Element::createMap();
     ElementPtr b = Element::createMap();
     ConstElementPtr c = Element::createMap();
-    ASSERT_NO_THROW(merge(a, b));
+    merge(a, b);
     EXPECT_EQ(*a, *c);
 
     a = Element::fromJSON("1");
@@ -858,102 +858,75 @@ TEST(Element, merge) {
     a = Element::createMap();
     b = Element::fromJSON("{ \"a\": 1 }");
     c = Element::fromJSON("{ \"a\": 1 }");
-    ASSERT_NO_THROW(merge(a, b));
+    merge(a, b);
     EXPECT_EQ(*a, *c);
 
     a = Element::createMap();
     b = Element::fromJSON("{ \"a\": 1 }");
     c = Element::fromJSON("{ \"a\": 1 }");
-    ASSERT_NO_THROW(merge(b, a));
+    merge(b, a);
     EXPECT_EQ(*b, *c);
 
     a = Element::fromJSON("{ \"a\": 1 }");
     b = Element::fromJSON("{ \"a\": 2 }");
     c = Element::fromJSON("{ \"a\": 2 }");
-    ASSERT_NO_THROW(merge(a, b));
+    merge(a, b);
     EXPECT_EQ(*a, *c);
 
     a = Element::fromJSON("{ \"a\": 1 }");
     b = Element::fromJSON("{ \"a\": 2 }");
     c = Element::fromJSON("{ \"a\": 1 }");
-    ASSERT_NO_THROW(merge(b, a));
+    merge(b, a);
     EXPECT_EQ(*b, *c);
 
     a = Element::fromJSON("{ \"a\": { \"b\": \"c\" } }");
     b = Element::fromJSON("{ \"a\": { \"b\": \"d\" } }");
     c = Element::fromJSON("{ \"a\": { \"b\": \"d\" } }");
-    ASSERT_NO_THROW(merge(a, b));
+    merge(a, b);
     EXPECT_EQ(*a, *c);
 
     a = Element::fromJSON("{ \"a\": { \"b\": \"c\" } }");
     b = Element::fromJSON("{ \"a\": { \"b\": \"d\" } }");
     c = Element::fromJSON("{ \"a\": { \"b\": \"c\" } }");
-    ASSERT_NO_THROW(merge(b, a));
+    merge(b, a);
     EXPECT_EQ(*b, *c);
 
     a = Element::fromJSON("{ \"a\": { \"b\": \"c\" } }");
     b = Element::fromJSON("{ \"a\": null }");
     c = Element::fromJSON("{  }");
-    ASSERT_NO_THROW(merge(a, b));
+    merge(a, b);
     EXPECT_EQ(*a, *c);
 
     a = Element::fromJSON("{ \"a\": { \"b\": \"c\" } }");
     b = Element::fromJSON("{ \"a\": null }");
     c = Element::fromJSON("{ \"a\": { \"b\": \"c\" } }");
-    ASSERT_NO_THROW(merge(b, a));
+    merge(b, a);
     EXPECT_EQ(*b, *c);
 
     // And some tests with multiple values
     a = Element::fromJSON("{ \"a\": 1, \"b\": true, \"c\": null }");
     b = Element::fromJSON("{ \"a\": 1, \"b\": null, \"c\": \"a string\" }");
     c = Element::fromJSON("{ \"a\": 1, \"c\": \"a string\" }");
-    ASSERT_NO_THROW(merge(a, b));
+    merge(a, b);
     EXPECT_EQ(*a, *c);
 
     a = Element::fromJSON("{ \"a\": 1, \"b\": true, \"c\": null }");
     b = Element::fromJSON("{ \"a\": 1, \"b\": null, \"c\": \"a string\" }");
     c = Element::fromJSON("{ \"a\": 1, \"b\": true }");
-    ASSERT_NO_THROW(merge(b, a));
+    merge(b, a);
     EXPECT_EQ(*b, *c);
 
     a = Element::fromJSON("{ \"a\": 1, \"b\": 2, \"c\": 3 }");
     b = Element::fromJSON("{ \"a\": 3, \"b\": 2, \"c\": 1 }");
     c = Element::fromJSON("{ \"a\": 3, \"b\": 2, \"c\": 1 }");
-    ASSERT_NO_THROW(merge(a, b));
+    merge(a, b);
     EXPECT_EQ(*a, *c);
 
     a = Element::fromJSON("{ \"a\": 1, \"b\": 2, \"c\": 3 }");
     b = Element::fromJSON("{ \"a\": 3, \"b\": 2, \"c\": 1 }");
     c = Element::fromJSON("{ \"a\": 1, \"b\": 2, \"c\": 3 }");
-    ASSERT_NO_THROW(merge(b, a));
+    merge(b, a);
     EXPECT_EQ(*b, *c);
 
-    // Map sub-elements: original map element is null
-    a = Element::fromJSON("{ \"a\": 1, \"m\": null }");
-    b = Element::fromJSON("{ \"a\": 3, \"m\": {  \"b\": 9 } }");
-    c = Element::fromJSON("{ \"a\": 3, \"m\": {  \"b\": 9 } }");
-    ASSERT_NO_THROW(merge(a, b));
-    EXPECT_EQ(*a, *c);
-
-    // Map sub-elements new map element has less elements than original
-    a = Element::fromJSON("{ \"a\": 1, \"m\": { \"b\": 2, \"c\": 3 } }");
-    b = Element::fromJSON("{ \"a\": 3, \"m\": { \"b\": 9 } }");
-    c = Element::fromJSON("{ \"a\": 3, \"m\": { \"b\": 9, \"c\": 3 } }");
-    ASSERT_NO_THROW(merge(a, b));
-    EXPECT_EQ(*a, *c);
-
-    // Map sub-elements new map element is null
-    a = Element::fromJSON("{ \"a\": 1, \"m\": { \"b\": 2, \"c\": 3 } }");
-    b = Element::fromJSON("{ \"a\": 3, \"m\": null }");
-    c = Element::fromJSON("{ \"a\": 3 }");
-    ASSERT_NO_THROW(merge(a, b));
-    EXPECT_EQ(*a, *c);
-
-    // Map sub-elements new map element has more elments than origina
-    a = Element::fromJSON("{ \"a\": 1, \"m\": { \"b\": 2, \"c\": 3 } }");
-    b = Element::fromJSON("{ \"a\": 3, \"m\": { \"b\": 2, \"c\": 3, \"d\": 4} }");
-    c = Element::fromJSON("{ \"a\": 3, \"m\": { \"b\": 2, \"c\": 3, \"d\": 4} }");
-    ASSERT_NO_THROW(merge(a, b));
-    EXPECT_EQ(*a, *c);
 }
 }

src/lib/python/isc/cc/data.py

@@ -1,4 +1,4 @@
-# Copyright (C) 2010-2014  Internet Systems Consortium.
+# Copyright (C) 2010  Internet Systems Consortium.
 #
 # Permission to use, copy, modify, and distribute this software for any
 # purpose with or without fee is hereby granted, provided that the above
@@ -51,24 +51,14 @@ def remove_identical(a, b):
         del(a[id])
 
 def merge(orig, new):
-    """Merges the contents of one dictionary into another.
-       The merge is done element by element, in order to recursivley merge
-       any elements which are themselves dictionaries. If an element value
-       is None in new it will be removed in orig. Previously this method
-       relied on dict.update but this does not do deep merges properly.
-       Raises a DataTypeError if either argument is not a dict"""
+    """Merges the contents of new into orig, think recursive update()
+       orig and new must both be dicts. If an element value is None in
+       new it will be removed in orig."""
     if type(orig) != dict or type(new) != dict:
         raise DataTypeError("Not a dict in merge()")
-
-    for key in new.keys():
-        if ((key in orig) and (type(orig[key]) == dict)):
-            merge(orig[key], new[key])
-        else:
-            orig[key] = new[key]
-
+    orig.update(new)
     remove_null_items(orig)
 
-
 def remove_null_items(d):
     """Recursively removes all (key,value) pairs from d where the
        value is None"""

src/lib/python/isc/cc/tests/data_test.py

@@ -34,37 +34,37 @@ class TestData(unittest.TestCase):
         c = {}
         data.remove_identical(a, b)
         self.assertEqual(a, c)
-
+    
         a = { "a": 1, "b": [ 1, 2 ] }
         b = {}
         c = { "a": 1, "b": [ 1, 2 ] }
         data.remove_identical(a, b)
         self.assertEqual(a, c)
-
+    
         a = { "a": 1, "b": [ 1, 2 ] }
         b = { "a": 1, "b": [ 1, 2 ] }
         c = {}
         data.remove_identical(a, b)
         self.assertEqual(a, c)
-
+    
         a = { "a": 1, "b": [ 1, 2 ] }
         b = { "a": 1, "b": [ 1, 3 ] }
         c = { "b": [ 1, 2 ] }
         data.remove_identical(a, b)
         self.assertEqual(a, c)
-
+    
         a = { "a": { "b": "c" } }
         b = {}
         c = { "a": { "b": "c" } }
         data.remove_identical(a, b)
         self.assertEqual(a, c)
-
+    
         a = { "a": { "b": "c" } }
         b = { "a": { "b": "c" } }
         c = {}
         data.remove_identical(a, b)
         self.assertEqual(a, c)
-
+    
         a = { "a": { "b": "c" } }
         b = { "a": { "b": "d" } }
         c = { "a": { "b": "c" } }
@@ -75,7 +75,7 @@ class TestData(unittest.TestCase):
                           a, 1)
         self.assertRaises(data.DataTypeError, data.remove_identical,
                           1, b)
-
+        
     def test_merge(self):
         d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2 } }
         d2 = { 'a': None, 'c': { 'd': None, 'e': 3, 'f': [ 1 ] } }
@@ -87,13 +87,6 @@ class TestData(unittest.TestCase):
         self.assertRaises(data.DataTypeError, data.merge, 1, d2)
         self.assertRaises(data.DataTypeError, data.merge, None, None)
 
-        # An example that failed when merge was relying on dict.update.
-        tnew = {'d2': {'port': 54000}}
-        torig = {'ifaces': ['p8p1'], 'db': {'type': 'memfile'}, 'd2': {'ip': '127.0.0.1', 'enable': True}}
-        tchk = {'ifaces': ['p8p1'], 'db': {'type': 'memfile'}, 'd2': {'ip': '127.0.0.1', 'enable': True, 'port': 54000}}
-        tmrg = torig
-        data.merge(tmrg, tnew)
-        self.assertEqual(tmrg, tchk)
 
     def test_split_identifier_list_indices(self):
         id, indices = data.split_identifier_list_indices('a')
@@ -110,15 +103,15 @@ class TestData(unittest.TestCase):
         id, indices = data.split_identifier_list_indices('a/b/c')
         self.assertEqual(id, 'a/b/c')
         self.assertEqual(indices, None)
-
+        
         id, indices = data.split_identifier_list_indices('a/b/c[1]')
         self.assertEqual(id, 'a/b/c')
         self.assertEqual(indices, [1])
-
+       
         id, indices = data.split_identifier_list_indices('a/b/c[1][2][3]')
         self.assertEqual(id, 'a/b/c')
         self.assertEqual(indices, [1, 2, 3])
-
+        
         id, indices = data.split_identifier_list_indices('a[0]/b[1]/c[2]')
         self.assertEqual(id, 'a[0]/b[1]/c')
         self.assertEqual(indices, [2])
@@ -131,7 +124,7 @@ class TestData(unittest.TestCase):
         self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a[0]a[1]')
 
         self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 1)
-
+        
 
     def test_find(self):
         d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2, 'more': { 'data': 'here' } } }
@@ -158,7 +151,7 @@ class TestData(unittest.TestCase):
         d3 = { 'a': [ { 'b': [ {}, { 'c': 'd' } ] } ] }
         self.assertEqual(data.find(d3, 'a[0]/b[1]/c'), 'd')
         self.assertRaises(data.DataNotFoundError, data.find, d3, 'a[1]/b[1]/c')
-
+        
     def test_set(self):
         d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2 } }
         d12 = { 'b': 1, 'c': { 'e': 3, 'f': [ 1 ] } }
@@ -177,7 +170,7 @@ class TestData(unittest.TestCase):
         self.assertEqual(d1, d14)
         data.set(d1, 'c/f[0]/g[1]', 3)
         self.assertEqual(d1, d15)
-
+        
         self.assertRaises(data.DataTypeError, data.set, d1, 1, 2)
         self.assertRaises(data.DataTypeError, data.set, 1, "", 2)
         self.assertRaises(data.DataTypeError, data.set, d1, 'c[1]', 2)
@@ -212,7 +205,7 @@ class TestData(unittest.TestCase):
         self.assertEqual(d3, { 'a': [ [ 1, 3 ] ] })
         data.unset(d3, 'a[0][1]')
         self.assertEqual(d3, { 'a': [ [ 1 ] ] })
-
+        
     def test_find_no_exc(self):
         d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2, 'more': { 'data': 'here' } } }
         self.assertEqual(data.find_no_exc(d1, ''), d1)
@@ -227,7 +220,7 @@ class TestData(unittest.TestCase):
         self.assertEqual(data.find_no_exc(None, 1), None)
         self.assertEqual(data.find_no_exc("123", ""), "123")
         self.assertEqual(data.find_no_exc("123", ""), "123")
-
+        
     def test_parse_value_str(self):
         self.assertEqual(data.parse_value_str("1"), 1)
         self.assertEqual(data.parse_value_str("true"), True)