merge branches/trac405

git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@3739 e5f2f494-b856-4b98-b285-d166d9295462

src/bin/bindctl/bindcmd.py

@@ -569,7 +569,10 @@ class BindCmdInterpreter(Cmd):
             elif cmd.command == "add":
                 self.config_data.add_value(identifier, cmd.params['value'])
             elif cmd.command == "remove":
-                self.config_data.remove_value(identifier, cmd.params['value'])
+                if 'value' in cmd.params:
+                    self.config_data.remove_value(identifier, cmd.params['value'])
+                else:
+                    self.config_data.remove_value(identifier, None)
             elif cmd.command == "set":
                 if 'identifier' not in cmd.params:
                     print("Error: missing identifier or value")

src/bin/bindctl/bindctl-source.py.in

@@ -51,7 +51,7 @@ def prepare_config_commands(tool):
     cmd = CommandInfo(name = "remove", desc = "Remove entry from configuration list")
     param = ParamInfo(name = "identifier", type = "string", optional=True)
     cmd.add_param(param)
-    param = ParamInfo(name = "value", type = "string", optional=False)
+    param = ParamInfo(name = "value", type = "string", optional=True)
     cmd.add_param(param)
     module.add_command(cmd)
 

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

@@ -56,20 +56,116 @@ def remove_null_items(d):
     for k in null_keys:
         del d[k]
 
-def find(element, identifier):
+def _concat_identifier(id_parts):
-    """Returns the subelement in the given data element, raises DataNotFoundError if not found"""
+    """Concatenates the given identifier parts into a string,
-    if type(identifier) != str or (type(element) != dict and identifier != ""):
+       delimited with the '/' character.
-        raise DataTypeError("identifier in merge() is not a string")
+    """
-    if type(identifier) != str or (type(element) != dict and identifier != ""):
+    return '/'.join(id_parts)
-        raise DataTypeError("element in merge() is not a dict")
+
-    id_parts = identifier.split("/")
+def split_identifier(identifier):
+    """Splits the given identifier into a list of identifier parts,
+       as delimited by the '/' character.
+       Raises a DataTypeError if identifier is not a string."""
+    if type(identifier) != str:
+        raise DataTypeError("identifier is not a string")
+    id_parts = identifier.split('/')
     id_parts[:] = (value for value in id_parts if value != "")
+    return id_parts
+
+def split_identifier_list_indices(identifier):
+    """Finds list indexes in the given identifier, which are of the
+       format [integer].
+       Identifier must be a string.
+       This will only give the list index for the last 'part' of the
+       given identifier (as delimited by the '/' sign).
+       Raises a DataTypeError if the identifier is not a string,
+       or if the format is bad.
+       Returns a tuple, where the first element is the string part of
+       the identifier, and the second element is a list of (nested) list
+       indices.
+       Examples:
+       'a/b/c' will return ('a/b/c', None)
+       'a/b/c[1]' will return ('a/b/c', [1])
+       'a/b/c[1][2][3]' will return ('a/b/c', [1, 2, 3])
+       'a[0]/b[1]/c[2]' will return ('a[0]/b[1]/c', [2])
+    """
+    if type(identifier) != str:
+        raise DataTypeError("identifier in "
+                            "split_identifier_list_indices() "
+                            "not a string: " + str(identifier))
+
+    # We only work on the final 'part' of the identifier
+    id_parts = split_identifier(identifier)
+    id_str = id_parts[-1]
+
+    i = id_str.find('[')
+    if i < 0:
+        if identifier.find(']') >= 0:
+            raise DataTypeError("Bad format in identifier: " + str(identifier))
+        return identifier, None
+
+    # keep the non-index part of that to replace later
+    id = id_str[:i]
+    indices = []
+    while i >= 0:
+        e = id_str.find(']')
+        if e < i + 1:
+            raise DataTypeError("Bad format in identifier: " + str(identifier))
+        try:
+            indices.append(int(id_str[i+1:e]))
+        except ValueError:
+            raise DataTypeError("List index in " + identifier + " not an integer")
+        id_str = id_str[e + 1:]
+        i = id_str.find('[')
+        if i > 0:
+            raise DataTypeError("Bad format in identifier: " + str(identifier))
+    if id.find(']') >= 0 or len(id_str) > 0:
+        raise DataTypeError("Bad format in identifier: " + str(identifier))
+
+    # we replace the final part of the original identifier with
+    # the stripped string
+    id_parts[-1] = id
+    id = _concat_identifier(id_parts)
+    return id, indices
+
+def _find_child_el(element, id):
+    """Finds the child of element with the given id. If the id contains
+       [i], where i is a number, and the child element is a list, the
+       i-th element of that list is returned instead of the list itself.
+       Raises a DataTypeError if the element is of wrong type, if id
+       is not a string, or if the id string contains a bad value.
+       Raises a DataNotFoundError if the element at id could not be
+       found.
+    """
+    id, list_indices = split_identifier_list_indices(id)
+    if type(element) == dict and id in element.keys():
+        result = element[id]
+    else:
+        raise DataNotFoundError(id + " in " + str(element))
+    if type(result) == list and list_indices is not None:
+        for list_index in list_indices:
+            if list_index >= len(result):
+                raise DataNotFoundError("Element " + str(list_index) + " in " + str(result))
+            result = result[list_index]
+    return result
+
+def find(element, identifier):
+    """Returns the subelement in the given data element, raises
+       DataNotFoundError if not found.
+       Returns the given element if the identifier is an empty string.
+       Raises a DataTypeError if identifier is not a string, or if
+       identifier is not empty, and element is not a dict.
+    """
+    if type(identifier) != str:
+        raise DataTypeError("identifier in find() is not a str")
+    if identifier == "":
+        return element
+    if type(element) != dict:
+        raise DataTypeError("element in find() is not a dict")
+    id_parts = split_identifier(identifier)
     cur_el = element
     for id in id_parts:
-        if type(cur_el) == dict and id in cur_el.keys():
+        cur_el = _find_child_el(cur_el, id)
-            cur_el = cur_el[id]
-        else:
-            raise DataNotFoundError(identifier + " in " + str(element))
     return cur_el
 
 def set(element, identifier, value):
@@ -83,25 +179,46 @@ def set(element, identifier, value):
     if type(element) != dict:
         raise DataTypeError("element in set() is not a dict")
     if type(identifier) != str:
-        raise DataTypeError("identifier in set() is not a string")
+        raise DataTypeError("identifier in set() is not a str")
-    id_parts = identifier.split("/")
+    id_parts = split_identifier(identifier)
-    id_parts[:] = (value for value in id_parts if value != "")
     cur_el = element
     for id in id_parts[:-1]:
-        if id in cur_el.keys():
+        try:
-            cur_el = cur_el[id]
+            cur_el = _find_child_el(cur_el, id)
-        else:
+        except DataNotFoundError:
-            if value == None:
+            if value is None:
                 # ok we are unsetting a value that wasn't set in
                 # the first place. Simply stop.
                 return
             cur_el[id] = {}
             cur_el = cur_el[id]
-    # value can be an empty list or dict, so check for None eplicitely
+
-    if value != None:
+    id, list_indices = split_identifier_list_indices(id_parts[-1])
-        cur_el[id_parts[-1]] = value
+    if list_indices is None:
-    elif id_parts[-1] in cur_el:
+        # value can be an empty list or dict, so check for None eplicitely
-        del cur_el[id_parts[-1]]
+        if value is not None:
+            cur_el[id] = value
+        else:
+            del cur_el[id]
+    else:
+        cur_el = cur_el[id]
+        # in case of nested lists, we need to get to the next to last
+        for list_index in list_indices[:-1]:
+            if type(cur_el) != list:
+                raise DataTypeError("Element at " + identifier + " is not a list")
+            if len(cur_el) <= list_index:
+                raise DataNotFoundError("List index at " + identifier + " out of range")
+            cur_el = cur_el[list_index]
+        # value can be an empty list or dict, so check for None eplicitely
+        list_index = list_indices[-1]
+        if type(cur_el) != list:
+            raise DataTypeError("Element at " + identifier + " is not a list")
+        if len(cur_el) <= list_index:
+            raise DataNotFoundError("List index at " + identifier + " out of range")
+        if value is not None:
+            cur_el[list_index] = value
+        else:
+            del cur_el[list_index]
     return element
 
 def unset(element, identifier):
@@ -116,17 +233,12 @@ def find_no_exc(element, identifier):
     """Returns the subelement in the given data element, returns None
        if not found, or if an error occurred (i.e. this function should
        never raise an exception)"""
-    if type(identifier) != str:
+    try:
+        return find(element, identifier)
+    except DataNotFoundError:
+        return None
+    except DataTypeError:
         return None
-    id_parts = identifier.split("/")
-    id_parts[:] = (value for value in id_parts if value != "")
-    cur_el = element
-    for id in id_parts:
-        if (type(cur_el) == dict and id in cur_el.keys()) or id=="":
-            cur_el = cur_el[id]
-        else:
-            return None
-    return cur_el
 
 def parse_value_str(value_str):
     """Parses the given string to a native python object. If the
@@ -139,7 +251,4 @@ def parse_value_str(value_str):
     except ValueError as ve:
         # simply return the string itself
         return value_str
-    except SyntaxError as ve:
-        # simply return the string itself
-        return value_str
 

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

@@ -70,6 +70,11 @@ class TestData(unittest.TestCase):
         c = { "a": { "b": "c" } }
         data.remove_identical(a, b)
         self.assertEqual(a, c)
+
+        self.assertRaises(data.DataTypeError, data.remove_identical,
+                          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 } }
@@ -82,6 +87,45 @@ class TestData(unittest.TestCase):
         self.assertRaises(data.DataTypeError, data.merge, 1, d2)
         self.assertRaises(data.DataTypeError, data.merge, None, None)
 
+
+    def test_split_identifier_list_indices(self):
+        id, indices = data.split_identifier_list_indices('a')
+        self.assertEqual(id, 'a')
+        self.assertEqual(indices, None)
+        id, indices = data.split_identifier_list_indices('a[0]')
+        self.assertEqual(id, 'a')
+        self.assertEqual(indices, [0])
+        id, indices = data.split_identifier_list_indices('a[0][1]')
+        self.assertEqual(id, 'a')
+        self.assertEqual(indices, [0, 1])
+
+        # examples from the docstring
+        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])
+
+        # bad formats
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a[')
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a]')
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a[[0]]')
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a[0]a')
+        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' } } }
         self.assertEqual(data.find(d1, ''), d1)
@@ -93,19 +137,47 @@ class TestData(unittest.TestCase):
         self.assertRaises(data.DataNotFoundError, data.find, d1, 'f')
         self.assertRaises(data.DataTypeError, data.find, d1, 1)
         self.assertRaises(data.DataTypeError, data.find, None, 1)
+        self.assertRaises(data.DataTypeError, data.find, None, "foo")
         self.assertRaises(data.DataTypeError, data.find, "123", "123")
         self.assertEqual(data.find("123", ""), "123")
+
+        d2 = { 'a': [ 1, 2, 3 ] }
+        self.assertEqual(data.find(d2, 'a[0]'), 1)
+        self.assertEqual(data.find(d2, 'a[1]'), 2)
+        self.assertEqual(data.find(d2, 'a[2]'), 3)
+        self.assertRaises(data.DataNotFoundError, data.find, d2, 'a[3]')
+        self.assertRaises(data.DataTypeError, data.find, d2, 'a[a]')
+
+        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 ] } }
+        d13 = { 'b': 1, 'c': { 'e': 3, 'f': [ 2 ] } }
+        d14 = { 'b': 1, 'c': { 'e': 3, 'f': [ { 'g': [ 1, 2 ] } ] } }
+        d15 = { 'b': 1, 'c': { 'e': 3, 'f': [ { 'g': [ 1, 3 ] } ] } }
         data.set(d1, 'a', None)
         data.set(d1, 'c/d', None)
         data.set(d1, 'c/e/', 3)
         data.set(d1, 'c/f', [ 1 ] )
         self.assertEqual(d1, d12)
+        data.set(d1, 'c/f[0]', 2 )
+        self.assertEqual(d1, d13)
+
+        data.set(d1, 'c/f[0]', { 'g': [ 1, 2] } )
+        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)
+        self.assertRaises(data.DataTypeError, data.set, d1, 'c[1][2]', 2)
+        self.assertRaises(data.DataNotFoundError, data.set, d1, 'c/f[5]', 2)
+        self.assertRaises(data.DataNotFoundError, data.set, d1, 'c/f[5][2]', 2)
+
         d3 = {}
         e3 = data.set(d3, "does/not/exist", 123)
         self.assertEqual(d3,
@@ -114,11 +186,25 @@ class TestData(unittest.TestCase):
                          { 'does': { 'not': { 'exist': 123 } } })
 
     def test_unset(self):
-        d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2 } }
+        d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': [ 1, 2, 3 ] } }
         data.unset(d1, 'a')
         data.unset(d1, 'c/d')
         data.unset(d1, 'does/not/exist')
-        self.assertEqual(d1, { 'b': 1, 'c': { 'e': 2 } })
+        self.assertEqual(d1, { 'b': 1, 'c': { 'e': [ 1, 2, 3 ] } })
+        data.unset(d1, 'c/e[0]')
+        self.assertEqual(d1, { 'b': 1, 'c': { 'e': [ 2, 3 ] } })
+        data.unset(d1, 'c/e[1]')
+        self.assertEqual(d1, { 'b': 1, 'c': { 'e': [ 2 ] } })
+        # index 1 should now be out of range
+        self.assertRaises(data.DataNotFoundError, data.unset, d1, 'c/e[1]')
+        d2 = { 'a': [ { 'b': [ 1, 2 ] } ] }
+        data.unset(d2, 'a[0]/b[1]')
+        self.assertEqual(d2, { 'a': [ { 'b': [ 1 ] } ] })
+        d3 = { 'a': [ [ 1, 2 ] ] }
+        data.set(d3, "a[0][1]", 3)
+        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' } } }
@@ -146,6 +232,9 @@ class TestData(unittest.TestCase):
         self.assertEqual(data.parse_value_str("{ \"a\": \"b\", \"c\": 1 }"), { 'a': 'b', 'c': 1 })
         self.assertEqual(data.parse_value_str("[ a c"), "[ a c")
 
+        self.assertEqual(data.parse_value_str(1), None)
+
+
 if __name__ == '__main__':
     #if not 'CONFIG_TESTDATA_PATH' in os.environ:
     #    print("You need to set the environment variable CONFIG_TESTDATA_PATH to point to the directory containing the test data files")

src/lib/python/isc/config/ccsession.py

@@ -398,16 +398,25 @@ class UIModuleCCSession(MultiConfigData):
         module_spec = self.find_spec_part(identifier)
         if (type(module_spec) != dict or "list_item_spec" not in module_spec):
             raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list")
-        value = isc.cc.data.parse_value_str(value_str)
+
-        isc.config.config_data.check_type(module_spec, [value])
+        if value_str is None:
-        cur_list, status = self.get_value(identifier)
+            # we are directly removing an list index
-        #if not cur_list:
+            id, list_indices = isc.cc.data.split_identifier_list_indices(identifier)
-        #    cur_list = isc.cc.data.find_no_exc(self.config.data, identifier)
+            if list_indices is None:
-        if not cur_list:
+                raise DataTypeError("identifier in remove_value() does not contain a list index, and no value to remove")
-            cur_list = []
+            else:
-        if value in cur_list:
+                self.set_value(identifier, None)
-            cur_list.remove(value)
+        else:
-        self.set_value(identifier, cur_list)
+            value = isc.cc.data.parse_value_str(value_str)
+            isc.config.config_data.check_type(module_spec, [value])
+            cur_list, status = self.get_value(identifier)
+            #if not cur_list:
+            #    cur_list = isc.cc.data.find_no_exc(self.config.data, identifier)
+            if not cur_list:
+                cur_list = []
+            if value in cur_list:
+                cur_list.remove(value)
+            self.set_value(identifier, cur_list)
 
     def commit(self):
         """Commit all local changes, send them through b10-cmdctl to

src/lib/python/isc/config/config_data.py

@@ -22,6 +22,7 @@ two through the classes in ccsession)
 
 import isc.cc.data
 import isc.config.module_spec
+import ast
 
 class ConfigDataError(Exception): pass
 
@@ -56,14 +57,14 @@ def check_type(spec_part, value):
         raise isc.cc.data.DataTypeError(str(value) + " is not a map")
 
 def convert_type(spec_part, value):
-    """Convert the give value(type is string) according specification 
+    """Convert the given value(type is string) according specification 
     part relevant for the value. Raises an isc.cc.data.DataTypeError 
     exception if conversion failed.
     """
     if type(spec_part) == dict and 'item_type' in spec_part:
         data_type = spec_part['item_type']
     else:
-        raise isc.cc.data.DataTypeError(str("Incorrect specification part for type convering"))
+        raise isc.cc.data.DataTypeError(str("Incorrect specification part for type conversion"))
    
     try:
         if data_type == "integer":
@@ -81,18 +82,25 @@ def convert_type(spec_part, value):
                     ret.append(convert_type(spec_part['list_item_spec'], item))
             elif type(value) == str:    
                 value = value.split(',')
-                for item in value:    
+                for item in value:
                     sub_value = item.split()
                     for sub_item in sub_value:
-                        ret.append(convert_type(spec_part['list_item_spec'], sub_item))
+                        ret.append(convert_type(spec_part['list_item_spec'],
+                                                sub_item))
 
             if ret == []:
                 raise isc.cc.data.DataTypeError(str(value) + " is not a list")
 
             return ret
         elif data_type == "map":
-            return dict(value)
+            map = ast.literal_eval(value)
-            # todo: check types of map contents too
+            if type(map) == dict:
+                # todo: check types of map contents too
+                return map
+            else:
+                raise isc.cc.data.DataTypeError(
+                           "Value in convert_type not a string "
+                           "specifying a dict")
         else:
             return value
     except ValueError as err:
@@ -108,7 +116,11 @@ def find_spec_part(element, identifier):
     id_parts = identifier.split("/")
     id_parts[:] = (value for value in id_parts if value != "")
     cur_el = element
-    for id in id_parts:
+
+    for id_part in id_parts:
+        # strip list selector part
+        # don't need it for the spec part, so just drop it
+        id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
         if type(cur_el) == dict and 'map_item_spec' in cur_el.keys():
             found = False
             for cur_el_item in cur_el['map_item_spec']:
@@ -226,6 +238,20 @@ class ConfigData:
             result[item] = value
         return result
 
+# should we just make a class for these?
+def _create_value_map_entry(name, type, value, status = None):
+    entry = {}
+    entry['name'] = name
+    entry['type'] = type
+    entry['value'] = value
+    entry['modified'] = False
+    entry['default'] = False
+    if status == MultiConfigData.LOCAL:
+        entry['modified'] = True
+    if status == MultiConfigData.DEFAULT:
+        entry['default'] = True
+    return entry
+
 class MultiConfigData:
     """This class stores the module specs, current non-default
        configuration values and 'local' (uncommitted) changes for
@@ -270,7 +296,7 @@ class MultiConfigData:
            identifier (up to the first /) is interpreted as the module
            name. Returns None if not found, or if identifier is not a
            string."""
-        if type(identifier) != str:
+        if type(identifier) != str or identifier == "":
             return None
         if identifier[0] == '/':
             identifier = identifier[1:]
@@ -334,28 +360,42 @@ class MultiConfigData:
         try:
             spec = find_spec_part(self._specifications[module].get_config_spec(), id)
             if 'item_default' in spec:
-                return spec['item_default']
+                id, list_indices = isc.cc.data.split_identifier_list_indices(id)
+                if list_indices is not None and \
+                   type(spec['item_default']) == list:
+                    if len(list_indices) == 1:
+                        default_list = spec['item_default']
+                        index = list_indices[0]
+                        if index < len(default_list):
+                            return default_list[index]
+                        else:
+                            return None
+                else:
+                    return spec['item_default']
             else:
                 return None
         except isc.cc.data.DataNotFoundError as dnfe:
             return None
 
-    def get_value(self, identifier):
+    def get_value(self, identifier, default = True):
         """Returns a tuple containing value,status.
            The value contains the configuration value for the given
            identifier. The status reports where this value came from;
            it is one of: LOCAL, CURRENT, DEFAULT or NONE, corresponding
            (local change, current setting, default as specified by the
-           specification, or not found at all)."""
+           specification, or not found at all). Does not check and
+           set DEFAULT if the argument 'default' is False (default
+           defaults to True)"""
         value = self.get_local_value(identifier)
         if value != None:
             return value, self.LOCAL
         value = self.get_current_value(identifier)
         if value != None:
             return value, self.CURRENT
-        value = self.get_default_value(identifier)
+        if default:
-        if value != None:
+            value = self.get_default_value(identifier)
-            return value, self.DEFAULT
+            if value != None:
+                return value, self.DEFAULT
         return None, self.NONE
 
     def get_value_maps(self, identifier = None):
@@ -372,12 +412,7 @@ class MultiConfigData:
         if not identifier:
             # No identifier, so we need the list of current modules
             for module in self._specifications.keys():
-                entry = {}
+                entry = _create_value_map_entry(module, 'module', None)
-                entry['name'] = module
-                entry['type'] = 'module'
-                entry['value'] = None
-                entry['modified'] = False
-                entry['default'] = False
                 result.append(entry)
         else:
             if identifier[0] == '/':
@@ -387,51 +422,41 @@ class MultiConfigData:
             if spec:
                 spec_part = find_spec_part(spec.get_config_spec(), id)
                 if type(spec_part) == list:
+                    # list of items to show
                     for item in spec_part:
-                        entry = {}
+                        value, status = self.get_value("/" + identifier\
-                        entry['name'] = item['item_name']
+                                              + "/" + item['item_name'])
-                        entry['type'] = item['item_type']
+                        entry = _create_value_map_entry(item['item_name'],
-                        value, status = self.get_value("/" + identifier + "/" + item['item_name'])
+                                                        item['item_type'],
-                        entry['value'] = value
+                                                        value, status)
-                        if status == self.LOCAL:
-                            entry['modified'] = True
-                        else:
-                            entry['modified'] = False
-                        if status == self.DEFAULT:
-                            entry['default'] = False
-                        else:
-                            entry['default'] = False
                         result.append(entry)
                 elif type(spec_part) == dict:
+                    # Sub-specification
                     item = spec_part
                     if item['item_type'] == 'list':
                         li_spec = item['list_item_spec']
-                        item_list, status =  self.get_value("/" + identifier)
+                        value, status =  self.get_value("/" + identifier)
-                        if item_list != None:
+                        if type(value) == list:
-                            for value in item_list:
+                            for list_value in value:
-                                result_part2 = {}
+                                result_part2 = _create_value_map_entry(
-                                result_part2['name'] = li_spec['item_name']
+                                                   li_spec['item_name'],
-                                result_part2['value'] = value
+                                                   li_spec['item_type'],
-                                result_part2['type'] = li_spec['item_type']
+                                                   list_value)
-                                result_part2['default'] = False
-                                result_part2['modified'] = False
                                 result.append(result_part2)
+                        elif value is not None:
+                            entry = _create_value_map_entry(
+                                        li_spec['item_name'],
+                                        li_spec['item_type'],
+                                        value, status)
+                            result.append(entry)
                     else:
-                        entry = {}
-                        entry['name'] = item['item_name']
-                        entry['type'] = item['item_type']
-                        #value, status = self.get_value("/" + identifier + "/" + item['item_name'])
                         value, status = self.get_value("/" + identifier)
-                        entry['value'] = value
+                        if value is not None:
-                        if status == self.LOCAL:
+                            entry = _create_value_map_entry(
-                            entry['modified'] = True
+                                        item['item_name'],
-                        else:
+                                        item['item_type'],
-                            entry['modified'] = False
+                                        value, status)
-                        if status == self.DEFAULT:
+                            result.append(entry)
-                            entry['default'] = False
-                        else:
-                            entry['default'] = False
-                        result.append(entry)
         return result
 
     def set_value(self, identifier, value):
@@ -439,8 +464,28 @@ class MultiConfigData:
            there is a specification for the given identifier, the type
            is checked."""
         spec_part = self.find_spec_part(identifier)
-        if spec_part != None:
+        if spec_part is not None and value is not None:
+            id, list_indices = isc.cc.data.split_identifier_list_indices(identifier)
+            if list_indices is not None \
+               and spec_part['item_type'] == 'list':
+                spec_part = spec_part['list_item_spec']
             check_type(spec_part, value)
+
+        # Since we do not support list diffs (yet?), we need to
+        # copy the currently set list of items to _local_changes
+        # if we want to modify an element in there
+        # (for any list indices specified in the full identifier)
+        id_parts = isc.cc.data.split_identifier(identifier)
+        cur_id_part = '/'
+        for id_part in id_parts:
+            id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
+            if list_indices is not None:
+                cur_list, status = self.get_value(cur_id_part + id)
+                if status != MultiConfigData.LOCAL:
+                    isc.cc.data.set(self._local_changes,
+                                    cur_id_part + id,
+                                    cur_list)
+            cur_id_part = cur_id_part + id_part + "/"
         isc.cc.data.set(self._local_changes, identifier, value)
  
     def get_config_item_list(self, identifier = None, recurse = False):

src/lib/python/isc/config/tests/ccsession_test.py

@@ -637,6 +637,8 @@ class TestUIModuleCCSession(unittest.TestCase):
         self.assertEqual({'Spec2': {'item5': ['foo']}}, uccs._local_changes)
         uccs.add_value("Spec2/item5", "foo")
         self.assertEqual({'Spec2': {'item5': ['foo']}}, uccs._local_changes)
+        uccs.remove_value("Spec2/item5[0]", None)
+        self.assertEqual({'Spec2': {'item5': []}}, uccs._local_changes)
 
     def test_commit(self):
         fake_conn = fakeUIConn()

src/lib/python/isc/config/tests/config_data_test.py

@@ -107,6 +107,8 @@ class TestConfigData(unittest.TestCase):
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "a")
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ 1, 2 ])
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, 1, "a")
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, { 'somedict': 'somevalue' }, "a")
         
         spec_part = find_spec_part(config_spec, "value2")
         self.assertEqual(1.1, convert_type(spec_part, '1.1'))
@@ -146,6 +148,18 @@ class TestConfigData(unittest.TestCase):
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "1", "b" ])
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
 
+        spec_part = find_spec_part(config_spec, "value6")
+        self.assertEqual({}, convert_type(spec_part, '{}'))
+        self.assertEqual({ 'v61': 'a' }, convert_type(spec_part, '{ \'v61\': \'a\' }'))
+
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, 1.1)
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, True)
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "a")
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "1")
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "a", "b" ])
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "1", "b" ])
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
+
         spec_part = find_spec_part(config_spec, "value7")
         self.assertEqual(['1', '2'], convert_type(spec_part, '1, 2'))
         self.assertEqual(['1', '2', '3'], convert_type(spec_part, '1 2  3'))
@@ -342,6 +356,12 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(1, value)
         value = self.mcd.get_default_value("/Spec2/item1")
         self.assertEqual(1, value)
+        value = self.mcd.get_default_value("Spec2/item5[0]")
+        self.assertEqual('a', value)
+        value = self.mcd.get_default_value("Spec2/item5[5]")
+        self.assertEqual(None, value)
+        value = self.mcd.get_default_value("Spec2/item5[0][1]")
+        self.assertEqual(None, value)
         value = self.mcd.get_default_value("Spec2/item6/value1")
         self.assertEqual('default', value)
         value = self.mcd.get_default_value("Spec2/item6/value2")
@@ -353,20 +373,34 @@ class TestMultiConfigData(unittest.TestCase):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
         self.mcd.set_specification(module_spec)
         self.mcd.set_value("Spec2/item1", 2)
-        value,status = self.mcd.get_value("Spec2/item1")
+
+        value, status = self.mcd.get_value("Spec2/item1")
         self.assertEqual(2, value)
         self.assertEqual(MultiConfigData.LOCAL, status)
-        value,status = self.mcd.get_value("Spec2/item2")
+
+        value, status = self.mcd.get_value("Spec2/item2")
         self.assertEqual(1.1, value)
         self.assertEqual(MultiConfigData.DEFAULT, status)
+
         self.mcd._current_config = { "Spec2": { "item3": False } }
-        value,status = self.mcd.get_value("Spec2/item3")
+
+        value, status = self.mcd.get_value("Spec2/item3")
         self.assertEqual(False, value)
         self.assertEqual(MultiConfigData.CURRENT, status)
-        value,status = self.mcd.get_value("Spec2/no_such_item")
+
+        value, status = self.mcd.get_value("Spec2/no_such_item")
         self.assertEqual(None, value)
         self.assertEqual(MultiConfigData.NONE, status)
 
+        value, status = self.mcd.get_value("Spec2/item5[0]")
+        self.assertEqual("a", value)
+        self.assertEqual(MultiConfigData.DEFAULT, status)
+
+        value, status = self.mcd.get_value("Spec2/item5[0]", False)
+        self.assertEqual(None, value)
+        self.assertEqual(MultiConfigData.NONE, status)
+
+
     def test_get_value_maps(self):
         maps = self.mcd.get_value_maps()
         self.assertEqual([], maps)
@@ -390,29 +424,31 @@ class TestMultiConfigData(unittest.TestCase):
         self.mcd.set_value("Spec2/item3", False)
         maps = self.mcd.get_value_maps("/Spec2")
         self.assertEqual([{'default': False, 'type': 'integer', 'name': 'item1', 'value': 2, 'modified': False},
-                          {'default': False, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
+                          {'default': True, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
                           {'default': False, 'type': 'boolean', 'name': 'item3', 'value': False, 'modified': True},
-                          {'default': False, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
+                          {'default': True, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
-                          {'default': False, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
+                          {'default': True, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
-                          {'default': False, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
+                          {'default': True, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("Spec2")
         self.assertEqual([{'default': False, 'type': 'integer', 'name': 'item1', 'value': 2, 'modified': False},
-                          {'default': False, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
+                          {'default': True, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
                           {'default': False, 'type': 'boolean', 'name': 'item3', 'value': False, 'modified': True},
-                          {'default': False, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
+                          {'default': True, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
-                          {'default': False, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
+                          {'default': True, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
-                          {'default': False, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
+                          {'default': True, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item5")
         self.assertEqual([{'default': False, 'type': 'string', 'name': 'list_element', 'value': 'a', 'modified': False},
                           {'default': False, 'type': 'string', 'name': 'list_element', 'value': 'b', 'modified': False}], maps)
+        maps = self.mcd.get_value_maps("/Spec2/item5[0]")
+        self.assertEqual([{'default': True, 'modified': False, 'name': 'list_element', 'type': 'string', 'value': 'a'}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item1")
         self.assertEqual([{'default': False, 'type': 'integer', 'name': 'item1', 'value': 2, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item2")
-        self.assertEqual([{'default': False, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False}], maps)
+        self.assertEqual([{'default': True, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item3")
         self.assertEqual([{'default': False, 'type': 'boolean', 'name': 'item3', 'value': False, 'modified': True}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item4")
-        self.assertEqual([{'default': False, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False}], maps)
+        self.assertEqual([{'default': True, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False}], maps)
 
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec24.spec")
         self.mcd.set_specification(module_spec)
@@ -429,7 +465,19 @@ class TestMultiConfigData(unittest.TestCase):
         self.mcd.set_specification(module_spec)
         self.mcd.set_value("Spec2/item1", 2)
         self.assertRaises(isc.cc.data.DataTypeError, self.mcd.set_value, "Spec2/item1", "asdf")
+
         self.mcd.set_value("Spec2/no_such_item", 4)
+        value, status = self.mcd.get_value("Spec2/no_such_item")
+        self.assertEqual(value, 4)
+        self.assertEqual(MultiConfigData.LOCAL, status)
+
+        self.mcd.set_value("Spec2/item5[0]", "c")
+        value, status = self.mcd.get_value("Spec2/item5[0]")
+        self.assertEqual(value, "c")
+        self.assertEqual(MultiConfigData.LOCAL, status)
+
+        self.assertRaises(isc.cc.data.DataTypeError, self.mcd.set_value, "Spec2/item5[a]", "asdf")
+        
 
     def test_get_config_item_list(self):
         config_items = self.mcd.get_config_item_list()
@@ -446,6 +494,8 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
         config_items = self.mcd.get_config_item_list("Spec2")
         self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6'], config_items)
+        config_items = self.mcd.get_config_item_list("/Spec2")
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6'], config_items)
         config_items = self.mcd.get_config_item_list("Spec2", True)
         self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)