Browse Source

Merge branch 'trac384'

Jelte Jansen 14 years ago
parent
commit
e5fb3bc1ed

+ 35 - 36
src/bin/auth/auth.spec.pre.in

@@ -12,45 +12,44 @@
         "item_type": "list",
         "item_type": "list",
         "item_optional": true,
         "item_optional": true,
         "item_default": [],
         "item_default": [],
-	"list_item_spec": {
+        "list_item_spec":
-          "item_name": "list_element",
+        { "item_name": "list_element",
           "item_type": "map",
           "item_type": "map",
           "item_optional": false,
           "item_optional": false,
           "item_default": {},
           "item_default": {},
-	  "map_item_spec": [
+          "map_item_spec": [
-	    { "item_name": "type",
+          { "item_name": "type",
-	      "item_type": "string",
+            "item_type": "string",
-	      "item_optional": false,
+            "item_optional": false,
-	      "item_default": ""
+            "item_default": ""
-	    },
+          },
-	    { "item_name": "class",
+          { "item_name": "class",
-	      "item_type": "string",
+            "item_type": "string",
-	      "item_optional": false,
+            "item_optional": false,
-	      "item_default": "IN"
+            "item_default": "IN"
-	    },
+          },
-	    { "item_name": "zones",
+          { "item_name": "zones",
-	      "item_type": "list",
+            "item_type": "list",
-	      "item_optional": false,
+            "item_optional": false,
-	      "item_default": [],
+            "item_default": [],
-	      "list_item_spec": {
+            "list_item_spec":
-	        "item_name": "list_element",
+            { "item_name": "list_element",
-	        "item_type": "map",
+              "item_type": "map",
-	        "item_optional": true,
+              "item_optional": true,
-	        "map_item_spec": [
+              "item_default": { "origin": "", "file": "" },
-		  { "item_name": "origin",
+              "map_item_spec": [
-		    "item_type": "string",
+              { "item_name": "origin",
-		    "item_optional": false,
+                "item_type": "string",
-		    "item_default": ""
+                "item_optional": false,
-		  },
+                "item_default": ""
-		  { "item_name": "file",
+              },
-		    "item_type": "string",
+              { "item_name": "file",
-		    "item_optional": false,
+                "item_type": "string",
-		    "item_default": ""
+                "item_optional": false,
-		  }
+                "item_default": ""
-		]
+              }]
-	      }
+            }
-	    }
+          }]
-	  ]
         }
         }
       },
       },
       { "item_name": "statistics-interval",
       { "item_name": "statistics-interval",

+ 153 - 88
src/bin/bindctl/bindcmd.py

@@ -51,7 +51,6 @@ except ImportError:
     my_readline = sys.stdin.readline
     my_readline = sys.stdin.readline
 
 
 CSV_FILE_NAME = 'default_user.csv'
 CSV_FILE_NAME = 'default_user.csv'
-FAIL_TO_CONNECT_WITH_CMDCTL = "Fail to connect with b10-cmdctl module, is it running?"
 CONFIG_MODULE_NAME = 'config'
 CONFIG_MODULE_NAME = 'config'
 CONST_BINDCTL_HELP = """
 CONST_BINDCTL_HELP = """
 usage: <module name> <command name> [param1 = value1 [, param2 = value2]]
 usage: <module name> <command name> [param1 = value1 [, param2 = value2]]
@@ -92,7 +91,10 @@ class BindCmdInterpreter(Cmd):
         Cmd.__init__(self)
         Cmd.__init__(self)
         self.location = ""
         self.location = ""
         self.prompt_end = '> '
         self.prompt_end = '> '
-        self.prompt = self.prompt_end
+        if sys.stdin.isatty():
+            self.prompt = self.prompt_end
+        else:
+            self.prompt = ""
         self.ruler = '-'
         self.ruler = '-'
         self.modules = OrderedDict()
         self.modules = OrderedDict()
         self.add_module_info(ModuleInfo("help", desc = "Get help for bindctl"))
         self.add_module_info(ModuleInfo("help", desc = "Get help for bindctl"))
@@ -119,8 +121,8 @@ class BindCmdInterpreter(Cmd):
 
 
             self.cmdloop()
             self.cmdloop()
         except FailToLogin as err:
         except FailToLogin as err:
-            print(err)
+            # error already printed when this was raised, ignoring
-            print(FAIL_TO_CONNECT_WITH_CMDCTL)
+            pass
         except KeyboardInterrupt:
         except KeyboardInterrupt:
             print('\nExit from bindctl')
             print('\nExit from bindctl')
 
 
@@ -270,8 +272,10 @@ class BindCmdInterpreter(Cmd):
         return line 
         return line 
 
 
     def postcmd(self, stop, line):
     def postcmd(self, stop, line):
-        '''Update the prompt after every command'''
+        '''Update the prompt after every command, but only if we
-        self.prompt = self.location + self.prompt_end
+           have a tty as output'''
+        if sys.stdin.isatty():
+            self.prompt = self.location + self.prompt_end
         return stop
         return stop
 
 
     def _prepare_module_commands(self, module_spec):
     def _prepare_module_commands(self, module_spec):
@@ -375,7 +379,14 @@ class BindCmdInterpreter(Cmd):
         if cmd.command == "help" or ("help" in cmd.params.keys()):
         if cmd.command == "help" or ("help" in cmd.params.keys()):
             self._handle_help(cmd)
             self._handle_help(cmd)
         elif cmd.module == CONFIG_MODULE_NAME:
         elif cmd.module == CONFIG_MODULE_NAME:
-            self.apply_config_cmd(cmd)
+            try:
+                self.apply_config_cmd(cmd)
+            except isc.cc.data.DataTypeError as dte:
+                print("Error: " + str(dte))
+            except isc.cc.data.DataNotFoundError as dnfe:
+                print("Error: " + str(dnfe))
+            except KeyError as ke:
+                print("Error: missing " + str(ke))
         else:
         else:
             self.apply_cmd(cmd)
             self.apply_cmd(cmd)
 
 
@@ -396,9 +407,24 @@ class BindCmdInterpreter(Cmd):
 
 
     def do_help(self, name):
     def do_help(self, name):
         print(CONST_BINDCTL_HELP)
         print(CONST_BINDCTL_HELP)
-        for k in self.modules.keys():
+        for k in self.modules.values():
-            print("\t", self.modules[k])
+            n = k.get_name()
-                
+            if len(n) >= CONST_BINDCTL_HELP_INDENT_WIDTH:
+                print("    %s" % n)
+                print(textwrap.fill(k.get_desc(),
+                      initial_indent="            ",
+                      subsequent_indent="    " +
+                      " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
+                      width=70))
+            else:
+                print(textwrap.fill("%s%s%s" %
+                    (k.get_name(),
+                     " "*(CONST_BINDCTL_HELP_INDENT_WIDTH - len(k.get_name())),
+                     k.get_desc()),
+                    initial_indent="    ",
+                    subsequent_indent="    " +
+                    " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
+                    width=70))
     
     
     def onecmd(self, line):
     def onecmd(self, line):
         if line == 'EOF' or line.lower() == "quit":
         if line == 'EOF' or line.lower() == "quit":
@@ -411,7 +437,19 @@ class BindCmdInterpreter(Cmd):
         Cmd.onecmd(self, line)
         Cmd.onecmd(self, line)
 
 
     def remove_prefix(self, list, prefix):
     def remove_prefix(self, list, prefix):
-        return [(val[len(prefix):]) for val in list]
+        """Removes the prefix already entered, and all elements from the
+           list that don't match it"""
+        if prefix.startswith('/'):
+            prefix = prefix[1:]
+
+        new_list = []
+        for val in list:
+            if val.startswith(prefix):
+                new_val = val[len(prefix):]
+                if new_val.startswith("/"):
+                    new_val = new_val[1:]
+                new_list.append(new_val)
+        return new_list
 
 
     def complete(self, text, state):
     def complete(self, text, state):
         if 0 == state:
         if 0 == state:
@@ -502,8 +540,7 @@ class BindCmdInterpreter(Cmd):
             self._validate_cmd(cmd)
             self._validate_cmd(cmd)
             self._handle_cmd(cmd)
             self._handle_cmd(cmd)
         except (IOError, http.client.HTTPException) as err:
         except (IOError, http.client.HTTPException) as err:
-            print('Error!', err)
+            print('Error: ', err)
-            print(FAIL_TO_CONNECT_WITH_CMDCTL)
         except BindCtlException as err:
         except BindCtlException as err:
             print("Error! ", err)
             print("Error! ", err)
             self._print_correct_usage(err)
             self._print_correct_usage(err)
@@ -541,87 +578,115 @@ class BindCmdInterpreter(Cmd):
            Raises a KeyError if the command was not complete
            Raises a KeyError if the command was not complete
         '''
         '''
         identifier = self.location
         identifier = self.location
-        try:
+        if 'identifier' in cmd.params:
-            if 'identifier' in cmd.params:
+            if not identifier.endswith("/"):
-                if not identifier.endswith("/"):
+                identifier += "/"
-                    identifier += "/"
+            if cmd.params['identifier'].startswith("/"):
-                if cmd.params['identifier'].startswith("/"):
+                identifier = cmd.params['identifier']
-                    identifier = cmd.params['identifier']
+            else:
-                else:
+                if cmd.params['identifier'].startswith('['):
-                    identifier += cmd.params['identifier']
+                    identifier = identifier[:-1]
-
+                identifier += cmd.params['identifier']
-                # Check if the module is known; for unknown modules
+
-                # we currently deny setting preferences, as we have
+            # Check if the module is known; for unknown modules
-                # no way yet to determine if they are ok.
+            # we currently deny setting preferences, as we have
-                module_name = identifier.split('/')[1]
+            # no way yet to determine if they are ok.
-                if self.config_data is None or \
+            module_name = identifier.split('/')[1]
-                   not self.config_data.have_specification(module_name):
+            if module_name != "" and (self.config_data is None or \
-                    print("Error: Module '" + module_name + "' unknown or not running")
+               not self.config_data.have_specification(module_name)):
-                    return
+                print("Error: Module '" + module_name + "' unknown or not running")
+                return
 
 
-            if cmd.command == "show":
+        if cmd.command == "show":
-                values = self.config_data.get_value_maps(identifier)
+            # check if we have the 'all' argument
-                for value_map in values:
+            show_all = False
-                    line = value_map['name']
+            if 'argument' in cmd.params:
-                    if value_map['type'] in [ 'module', 'map', 'list' ]:
+                if cmd.params['argument'] == 'all':
-                        line += "/"
+                    show_all = True
-                    else:
+                elif 'identifier' not in cmd.params:
-                        line += ":\t" + json.dumps(value_map['value'])
+                    # no 'all', no identifier, assume this is the
-                    line += "\t" + value_map['type']
+                    #identifier
-                    line += "\t"
+                    identifier += cmd.params['argument']
-                    if value_map['default']:
-                        line += "(default)"
-                    if value_map['modified']:
-                        line += "(modified)"
-                    print(line)
-            elif cmd.command == "add":
-                self.config_data.add_value(identifier, cmd.params['value'])
-            elif cmd.command == "remove":
-                if 'value' in cmd.params:
-                    self.config_data.remove_value(identifier, cmd.params['value'])
                 else:
                 else:
-                    self.config_data.remove_value(identifier, None)
+                    print("Error: unknown argument " + cmd.params['argument'] + ", or multiple identifiers given")
-            elif cmd.command == "set":
+                    return
-                if 'identifier' not in cmd.params:
+            values = self.config_data.get_value_maps(identifier, show_all)
-                    print("Error: missing identifier or value")
+            for value_map in values:
+                line = value_map['name']
+                if value_map['type'] in [ 'module', 'map' ]:
+                    line += "/"
+                elif value_map['type'] == 'list' \
+                     and value_map['value'] != []:
+                    # do not print content of non-empty lists if
+                    # we have more data to show
+                    line += "/"
                 else:
                 else:
-                    parsed_value = None
+                    line += "\t" + json.dumps(value_map['value'])
-                    try:
+                line += "\t" + value_map['type']
-                        parsed_value = json.loads(cmd.params['value'])
+                line += "\t"
-                    except Exception as exc:
+                if value_map['default']:
-                        # ok could be an unquoted string, interpret as such
+                    line += "(default)"
-                        parsed_value = cmd.params['value']
+                if value_map['modified']:
-                    self.config_data.set_value(identifier, parsed_value)
+                    line += "(modified)"
-            elif cmd.command == "unset":
+                print(line)
-                self.config_data.unset(identifier)
+        elif cmd.command == "show_json":
-            elif cmd.command == "revert":
+            if identifier == "":
-                self.config_data.clear_local_changes()
+                print("Need at least the module to show the configuration in JSON format")
-            elif cmd.command == "commit":
+            else:
-                self.config_data.commit()
+                data, default = self.config_data.get_value(identifier)
-            elif cmd.command == "diff":
+                print(json.dumps(data))
-                print(self.config_data.get_local_changes());
+        elif cmd.command == "add":
-            elif cmd.command == "go":
+            if 'value' in cmd.params:
-                self.go(identifier)
+                self.config_data.add_value(identifier, cmd.params['value'])
-        except isc.cc.data.DataTypeError as dte:
+            else:
-            print("Error: " + str(dte))
+                self.config_data.add_value(identifier)
-        except isc.cc.data.DataNotFoundError as dnfe:
+        elif cmd.command == "remove":
-            print("Error: " + identifier + " not found")
+            if 'value' in cmd.params:
-        except KeyError as ke:
+                self.config_data.remove_value(identifier, cmd.params['value'])
-            print("Error: missing " + str(ke))
+            else:
-            raise ke
+                self.config_data.remove_value(identifier, None)
+        elif cmd.command == "set":
+            if 'identifier' not in cmd.params:
+                print("Error: missing identifier or value")
+            else:
+                parsed_value = None
+                try:
+                    parsed_value = json.loads(cmd.params['value'])
+                except Exception as exc:
+                    # ok could be an unquoted string, interpret as such
+                    parsed_value = cmd.params['value']
+                self.config_data.set_value(identifier, parsed_value)
+        elif cmd.command == "unset":
+            self.config_data.unset(identifier)
+        elif cmd.command == "revert":
+            self.config_data.clear_local_changes()
+        elif cmd.command == "commit":
+            self.config_data.commit()
+        elif cmd.command == "diff":
+            print(self.config_data.get_local_changes());
+        elif cmd.command == "go":
+            self.go(identifier)
 
 
     def go(self, identifier):
     def go(self, identifier):
         '''Handles the config go command, change the 'current' location
         '''Handles the config go command, change the 'current' location
-           within the configuration tree'''
+           within the configuration tree. '..' will be interpreted as
-        # this is just to see if it exists
+           'up one level'.'''
-        self.config_data.get_value(identifier)
+        id_parts = isc.cc.data.split_identifier(identifier)
-        # some sanitizing
+
-        identifier = identifier.replace("//", "/")
+        new_location = ""
-        if not identifier.startswith("/"):
+        for id_part in id_parts:
-            identifier = "/" + identifier
+            if (id_part == ".."):
-        if identifier.endswith("/"):
+                # go 'up' one level
-            identifier = identifier[:-1]
+                new_location, a, b = new_location.rpartition("/")
-        self.location = identifier
+            else:
+                new_location += "/" + id_part
+        # check if exists, if not, revert and error
+        v,d = self.config_data.get_value(new_location)
+        if v is None:
+            print("Error: " + identifier + " not found")
+            return
+
+        self.location = new_location
 
 
     def apply_cmd(self, cmd):
     def apply_cmd(self, cmd):
         '''Handles a general module command'''
         '''Handles a general module command'''

+ 29 - 23
src/bin/bindctl/bindctl-source.py.in

@@ -33,51 +33,60 @@ isc.util.process.rename()
 # number, and the overall BIND 10 version number (set in configure.ac).
 # number, and the overall BIND 10 version number (set in configure.ac).
 VERSION = "bindctl 20101201 (BIND 10 @PACKAGE_VERSION@)"
 VERSION = "bindctl 20101201 (BIND 10 @PACKAGE_VERSION@)"
 
 
+DEFAULT_IDENTIFIER_DESC = "The identifier specifiec the config item. Child elements are separated with the '/' character. List indices can be specified with '[i]', where i is an integer specifying the index, starting with 0. Examples: 'Boss/start_auth', 'Recurse/listen_on[0]/address'. If no identifier is given, shows the item at the current location."
+
 def prepare_config_commands(tool):
 def prepare_config_commands(tool):
     '''Prepare fixed commands for local configuration editing'''
     '''Prepare fixed commands for local configuration editing'''
     module = ModuleInfo(name = CONFIG_MODULE_NAME, desc = "Configuration commands")
     module = ModuleInfo(name = CONFIG_MODULE_NAME, desc = "Configuration commands")
     cmd = CommandInfo(name = "show", desc = "Show configuration")
     cmd = CommandInfo(name = "show", desc = "Show configuration")
-    param = ParamInfo(name = "identifier", type = "string", optional=True)
+    param = ParamInfo(name = "argument", type = "string", optional=True, desc = "If you specify the argument 'all' (before the identifier), recursively shows all child elements for the given identifier")
+    cmd.add_param(param)
+    param = ParamInfo(name = "identifier", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC)
+    cmd.add_param(param)
+    module.add_command(cmd)
+
+    cmd = CommandInfo(name = "show_json", desc = "Show full configuration in JSON format")
+    param = ParamInfo(name = "identifier", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     cmd.add_param(param)
     module.add_command(cmd)
     module.add_command(cmd)
 
 
-    cmd = CommandInfo(name = "add", desc = "Add entry to configuration list")
+    cmd = CommandInfo(name = "add", desc = "Add an entry to configuration list. If no value is given, a default value is added.")
-    param = ParamInfo(name = "identifier", type = "string", optional=True)
+    param = ParamInfo(name = "identifier", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     cmd.add_param(param)
-    param = ParamInfo(name = "value", type = "string", optional=False)
+    param = ParamInfo(name = "value", type = "string", optional=True, desc = "Specifies a value to add to the list. It must be in correct JSON format and complete.")
     cmd.add_param(param)
     cmd.add_param(param)
     module.add_command(cmd)
     module.add_command(cmd)
 
 
     cmd = CommandInfo(name = "remove", desc = "Remove entry from configuration list")
     cmd = CommandInfo(name = "remove", desc = "Remove entry from configuration list")
-    param = ParamInfo(name = "identifier", type = "string", optional=True)
+    param = ParamInfo(name = "identifier", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     cmd.add_param(param)
-    param = ParamInfo(name = "value", type = "string", optional=True)
+    param = ParamInfo(name = "value", type = "string", optional=True, desc = "Specifies a value to remove from the list. It must be in correct JSON format and complete.")
     cmd.add_param(param)
     cmd.add_param(param)
     module.add_command(cmd)
     module.add_command(cmd)
 
 
     cmd = CommandInfo(name = "set", desc = "Set a configuration value")
     cmd = CommandInfo(name = "set", desc = "Set a configuration value")
-    param = ParamInfo(name = "identifier", type = "string", optional=True)
+    param = ParamInfo(name = "identifier", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     cmd.add_param(param)
-    param = ParamInfo(name = "value", type = "string", optional=False)
+    param = ParamInfo(name = "value", type = "string", optional=False, desc = "Specifies a value to set. It must be in correct JSON format and complete.")
     cmd.add_param(param)
     cmd.add_param(param)
     module.add_command(cmd)
     module.add_command(cmd)
 
 
-    cmd = CommandInfo(name = "unset", desc = "Unset a configuration value")
+    cmd = CommandInfo(name = "unset", desc = "Unset a configuration value (i.e. revert to the default, if any)")
-    param = ParamInfo(name = "identifier", type = "string", optional=False)
+    param = ParamInfo(name = "identifier", type = "string", optional=False, desc = DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     cmd.add_param(param)
     module.add_command(cmd)
     module.add_command(cmd)
 
 
-    cmd = CommandInfo(name = "diff", desc = "Show all local changes")
+    cmd = CommandInfo(name = "diff", desc = "Show all local changes that have not been committed")
     module.add_command(cmd)
     module.add_command(cmd)
 
 
     cmd = CommandInfo(name = "revert", desc = "Revert all local changes")
     cmd = CommandInfo(name = "revert", desc = "Revert all local changes")
     module.add_command(cmd)
     module.add_command(cmd)
 
 
-    cmd = CommandInfo(name = "commit", desc = "Commit all local changes")
+    cmd = CommandInfo(name = "commit", desc = "Commit all local changes.")
     module.add_command(cmd)
     module.add_command(cmd)
 
 
     cmd = CommandInfo(name = "go", desc = "Go to a specific configuration part")
     cmd = CommandInfo(name = "go", desc = "Go to a specific configuration part")
-    param = ParamInfo(name = "identifier", type="string", optional=False)
+    param = ParamInfo(name = "identifier", type="string", optional=False, desc = DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     cmd.add_param(param)
     module.add_command(cmd)
     module.add_command(cmd)
 
 
@@ -115,15 +124,12 @@ def set_bindctl_options(parser):
                       help = 'PEM formatted server certificate validation chain file')
                       help = 'PEM formatted server certificate validation chain file')
 
 
 if __name__ == '__main__':
 if __name__ == '__main__':
-    try:
+    parser = OptionParser(version = VERSION)
-        parser = OptionParser(version = VERSION)
+    set_bindctl_options(parser)
-        set_bindctl_options(parser)
+    (options, args) = parser.parse_args()
-        (options, args) = parser.parse_args()
+    server_addr = options.addr + ':' + str(options.port)
-        server_addr = options.addr + ':' + str(options.port)
+    tool = BindCmdInterpreter(server_addr, pem_file=options.cert_chain)
-        tool = BindCmdInterpreter(server_addr, pem_file=options.cert_chain)
+    prepare_config_commands(tool)
-        prepare_config_commands(tool)
+    tool.run()
-        tool.run()
-    except Exception as e:
-        print(e, "\nFailed to connect with b10-cmdctl module, is it running?")
 
 
 
 

+ 54 - 0
src/bin/bindctl/cmdparse.py

@@ -33,6 +33,7 @@ param_value_str  = "(?P<param_value>[^\'\" ][^, ]+)"
 param_value_with_quota_str  = "[\"\'](?P<param_value>.+?)(?<!\\\)[\"\']"
 param_value_with_quota_str  = "[\"\'](?P<param_value>.+?)(?<!\\\)[\"\']"
 next_params_str = "(?P<blank>\s*)(?P<comma>,?)(?P<next_params>.*)$"
 next_params_str = "(?P<blank>\s*)(?P<comma>,?)(?P<next_params>.*)$"
 
 
+
 PARAM_WITH_QUOTA_PATTERN = re.compile(param_name_str + 
 PARAM_WITH_QUOTA_PATTERN = re.compile(param_name_str + 
                                       param_value_with_quota_str + 
                                       param_value_with_quota_str + 
                                       next_params_str)
                                       next_params_str)
@@ -40,6 +41,56 @@ PARAM_PATTERN = re.compile(param_name_str + param_value_str + next_params_str)
 # Used for module and command name
 # Used for module and command name
 NAME_PATTERN = re.compile("^\s*(?P<name>[\w]+)(?P<blank>\s*)(?P<others>.*)$")
 NAME_PATTERN = re.compile("^\s*(?P<name>[\w]+)(?P<blank>\s*)(?P<others>.*)$")
 
 
+# this removes all whitespace inthe given string, except when
+# between " quotes
+_remove_unquoted_whitespace = \
+    lambda text:'"'.join( it if i%2 else ''.join(it.split())
+        for i,it in enumerate(text.split('"'))  )
+
+
+def _remove_list_and_map_whitespace(text):
+    """Returns a string where the whitespace between matching [ and ]
+       is removed, unless quoted"""
+    # regular expression aren't really the right tool, since we may have
+    # nested structures
+    result = []
+    start_pos = 0
+    pos = 0
+    list_count = 0
+    map_count = 0
+    cur_start_list_pos = None
+    cur_start_map_pos = None
+    for i in text:
+        if i == '[' and map_count == 0:
+            if list_count == 0:
+                result.append(text[start_pos:pos + 1])
+                cur_start_list_pos = pos + 1
+            list_count = list_count + 1
+        elif i == ']' and map_count == 0:
+            if list_count > 0:
+                list_count = list_count - 1
+                if list_count == 0:
+                    result.append(_remove_unquoted_whitespace(text[cur_start_list_pos:pos + 1]))
+                    start_pos = pos + 1
+        if i == '{' and list_count == 0:
+            if map_count == 0:
+                result.append(text[start_pos:pos + 1])
+                cur_start_map_pos = pos + 1
+            map_count = map_count + 1
+        elif i == '}' and list_count == 0:
+            if map_count > 0:
+                map_count = map_count - 1
+                if map_count == 0:
+                    result.append(_remove_unquoted_whitespace(text[cur_start_map_pos:pos + 1]))
+                    start_pos = pos + 1
+        
+
+        pos = pos + 1
+    if start_pos <= len(text):
+        result.append(text[start_pos:len(text)])
+    return "".join(result)
+    
+    
 class BindCmdParse:
 class BindCmdParse:
     """ This class will parse the command line usr input into three part
     """ This class will parse the command line usr input into three part
     module name, command, parameters
     module name, command, parameters
@@ -86,9 +137,12 @@ class BindCmdParse:
 
 
             self._parse_params(param_str)
             self._parse_params(param_str)
 
 
+    def _remove_list_whitespace(self, text):
+        return ""
 
 
     def _parse_params(self, param_text):
     def _parse_params(self, param_text):
         """convert a=b,c=d into one hash """
         """convert a=b,c=d into one hash """
+        param_text = _remove_list_and_map_whitespace(param_text)
         
         
         # Check parameter name "help"
         # Check parameter name "help"
         param = NAME_PATTERN.match(param_text)
         param = NAME_PATTERN.match(param_text)

+ 55 - 9
src/bin/bindctl/moduleinfo.py

@@ -16,6 +16,8 @@
 """This module holds classes representing modules, commands and
 """This module holds classes representing modules, commands and
    parameters for use in bindctl"""
    parameters for use in bindctl"""
 
 
+import textwrap
+
 try:
 try:
     from collections import OrderedDict
     from collections import OrderedDict
 except ImportError:
 except ImportError:
@@ -30,6 +32,9 @@ MODULE_NODE_NAME = 'module'
 COMMAND_NODE_NAME = 'command'
 COMMAND_NODE_NAME = 'command'
 PARAM_NODE_NAME = 'param'
 PARAM_NODE_NAME = 'param'
 
 
+# this is used to align the descriptions in help output
+CONST_BINDCTL_HELP_INDENT_WIDTH=12
+
 
 
 class ParamInfo:
 class ParamInfo:
     """One parameter of one command.
     """One parameter of one command.
@@ -52,6 +57,12 @@ class ParamInfo:
     def __str__(self):        
     def __str__(self):        
         return str("\t%s <type: %s> \t(%s)" % (self.name, self.type, self.desc))
         return str("\t%s <type: %s> \t(%s)" % (self.name, self.type, self.desc))
 
 
+    def get_name(self):
+        return "%s <type: %s>" % (self.name, self.type)
+
+    def get_desc(self):
+        return self.desc
+
 class CommandInfo:
 class CommandInfo:
     """One command which is provided by one bind10 module, it has zero
     """One command which is provided by one bind10 module, it has zero
        or more parameters
        or more parameters
@@ -68,8 +79,13 @@ class CommandInfo:
                 
                 
     def __str__(self):
     def __str__(self):
         return str("%s \t(%s)" % (self.name, self.desc))
         return str("%s \t(%s)" % (self.name, self.desc))
-        
 
 
+    def get_name(self):
+        return self.name
+
+    def get_desc(self):
+        return self.desc;
+    
     def add_param(self, paraminfo):
     def add_param(self, paraminfo):
         """Add a ParamInfo object to this CommandInfo"""
         """Add a ParamInfo object to this CommandInfo"""
         self.params[paraminfo.name] = paraminfo
         self.params[paraminfo.name] = paraminfo
@@ -144,22 +160,30 @@ class CommandInfo:
         del params["help"]
         del params["help"]
 
 
         if len(params) == 0:
         if len(params) == 0:
-            print("\tNo parameters for the command")
+            print("No parameters for the command")
             return
             return
         
         
-        print("\n\tMandatory parameters:")
+        print("\nMandatory parameters:")
         mandatory_infos = []
         mandatory_infos = []
         for info in params.values():            
         for info in params.values():            
             if not info.is_optional:
             if not info.is_optional:
-                print("\t", info)
+                print("    %s" % info.get_name())
+                print(textwrap.fill(info.get_desc(),
+                      initial_indent="        ",
+                      subsequent_indent="        ",
+                      width=70))
                 mandatory_infos.append(info)
                 mandatory_infos.append(info)
 
 
         optional_infos = [info for info in params.values() 
         optional_infos = [info for info in params.values() 
                           if info not in mandatory_infos]
                           if info not in mandatory_infos]
         if len(optional_infos) > 0:
         if len(optional_infos) > 0:
-            print("\n\tOptional parameters:")      
+            print("\nOptional parameters:")      
             for info in optional_infos:
             for info in optional_infos:
-                    print("\t", info)
+                print("    %s" % info.get_name())
+                print(textwrap.fill(info.get_desc(),
+                      initial_indent="        ",
+                      subsequent_indent="        ",
+                      width=70))
 
 
 
 
 class ModuleInfo:
 class ModuleInfo:
@@ -176,7 +200,13 @@ class ModuleInfo:
         
         
     def __str__(self):
     def __str__(self):
         return str("%s \t%s" % (self.name, self.desc))
         return str("%s \t%s" % (self.name, self.desc))
-        
+
+    def get_name(self):
+        return self.name
+
+    def get_desc(self):
+        return self.desc
+
     def add_command(self, command_info):
     def add_command(self, command_info):
         """Add a CommandInfo to this ModuleInfo."""
         """Add a CommandInfo to this ModuleInfo."""
         self.commands[command_info.name] = command_info
         self.commands[command_info.name] = command_info
@@ -201,8 +231,24 @@ class ModuleInfo:
     def module_help(self):
     def module_help(self):
         """Prints the help info for this module to stdout"""
         """Prints the help info for this module to stdout"""
         print("Module ", self, "\nAvailable commands:")
         print("Module ", self, "\nAvailable commands:")
-        for k in self.commands.keys():
+        for k in self.commands.values():
-            print("\t", self.commands[k])
+            n = k.get_name()
+            if len(n) >= CONST_BINDCTL_HELP_INDENT_WIDTH:
+                print("    %s" % n)
+                print(textwrap.fill(k.get_desc(),
+                      initial_indent="            ",
+                      subsequent_indent="    " +
+                      " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
+                      width=70))
+            else:
+                print(textwrap.fill("%s%s%s" %
+                    (k.get_name(),
+                     " "*(CONST_BINDCTL_HELP_INDENT_WIDTH - len(k.get_name())),
+                     k.get_desc()),
+                    initial_indent="    ",
+                    subsequent_indent="    " +
+                    " " * CONST_BINDCTL_HELP_INDENT_WIDTH,
+                    width=70))
             
             
     def command_help(self, command):
     def command_help(self, command):
         """Prints the help info for the command with the given name.
         """Prints the help info for the command with the given name.

+ 1 - 1
src/bin/bindctl/tests/Makefile.am

@@ -1,5 +1,5 @@
 PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
 PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
-PYTESTS = bindctl_test.py
+PYTESTS = bindctl_test.py cmdparse_test.py
 EXTRA_DIST = $(PYTESTS)
 EXTRA_DIST = $(PYTESTS)
 
 
 # test using command-line arguments, so use check-local target instead of TESTS
 # test using command-line arguments, so use check-local target instead of TESTS

+ 94 - 2
src/bin/bindctl/tests/bindctl_test.py

@@ -17,6 +17,8 @@
 import unittest
 import unittest
 import isc.cc.data
 import isc.cc.data
 import os
 import os
+from isc.config.config_data import ConfigData, MultiConfigData
+from isc.config.module_spec import ModuleSpec
 from bindctl import cmdparse
 from bindctl import cmdparse
 from bindctl import bindcmd
 from bindctl import bindcmd
 from bindctl.moduleinfo import *
 from bindctl.moduleinfo import *
@@ -238,11 +240,101 @@ class TestNameSequence(unittest.TestCase):
             assert self.random_names[i] == module_names[i+1]
             assert self.random_names[i] == module_names[i+1]
             i = i + 1
             i = i + 1
 
 
-    def test_apply_cfg_command(self):
+# tine class to fake a UIModuleCCSession, but only the config data
+# parts for the next set of tests
+class FakeCCSession(MultiConfigData):
+    def __init__(self):
+        self._local_changes = {}
+        self._current_config = {}
+        self._specifications = {}
+        self.add_foo_spec()
+
+    def add_foo_spec(self):
+        spec = { "module_name": "foo",
+                 "config_data": [
+                 { "item_name": "an_int",
+                   "item_type": "integer",
+                   "item_optional": False,
+                   "item_default": 1
+                 },
+                 { "item_name": "a_list",
+                   "item_type": "list",
+                   "item_optional": False,
+                   "item_default": [],
+                   "list_item_spec":
+                   { "item_name": "a_string",
+                     "item_type": "string",
+                     "item_optional": False,
+                     "item_default": "bar"
+                   }
+                 }
+                 ]
+               }
+        self.set_specification(ModuleSpec(spec))
+    
+
+class TestConfigCommands(unittest.TestCase):
+    def setUp(self):
+        self.tool = bindcmd.BindCmdInterpreter()
+        mod_info = ModuleInfo(name = "foo")
+        self.tool.add_module_info(mod_info)
+        self.tool.config_data = FakeCCSession()
+        
+    def test_apply_cfg_command_int(self):
         self.tool.location = '/'
         self.tool.location = '/'
-        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/bar\" value=\"5\"")
+
+        self.assertEqual((1, MultiConfigData.DEFAULT),
+                         self.tool.config_data.get_value("/foo/an_int"))
+
+        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/an_int\" value=\"5\"")
         self.tool.apply_config_cmd(cmd)
         self.tool.apply_config_cmd(cmd)
+        self.assertEqual((5, MultiConfigData.LOCAL),
+                         self.tool.config_data.get_value("/foo/an_int"))
+
+        # this should raise a NotFoundError
+        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/bar\" value=\"[]\"")
+        self.assertRaises(isc.cc.data.DataNotFoundError, self.tool.apply_config_cmd, cmd)
+
+        # this should raise a TypeError
+        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/an_int\" value=\"[]\"")
+        self.assertRaises(isc.cc.data.DataTypeError, self.tool.apply_config_cmd, cmd)
+
+    # this is a very specific one for use with a set of list tests
+    # to try out the flexibility of the parser (only in the next test)
+    def clt(self, full_cmd_string, item_value):
+        cmd = cmdparse.BindCmdParse(full_cmd_string)
+        self.tool.apply_config_cmd(cmd)
+        self.assertEqual(([item_value], MultiConfigData.LOCAL),
+                         self.tool.config_data.get_value("/foo/a_list"))
+
+    def test_apply_cfg_command_list(self):
+        self.tool.location = '/'
+
+        self.assertEqual(([], MultiConfigData.DEFAULT),
+                         self.tool.config_data.get_value("/foo/a_list"))
+
+        self.clt("config set identifier=\"foo/a_list\" value=[\"a\"]", "a")
+        self.clt("config set identifier=\"foo/a_list\" value =[\"b\"]", "b")
+        self.clt("config set identifier=\"foo/a_list\" value= [\"c\"]", "c")
+        self.clt("config set identifier=\"foo/a_list\" value = [\"d\"]", "d")
+        self.clt("config set identifier =\"foo/a_list\" value=[\"e\"]", "e")
+        self.clt("config set identifier= \"foo/a_list\" value=[\"f\"]", "f")
+        self.clt("config set identifier = \"foo/a_list\" value=[\"g\"]", "g")
+        self.clt("config set identifier = \"foo/a_list\" value = [\"h\"]", "h")
+        self.clt("config set identifier = \"foo/a_list\" value=[\"i\" ]", "i")
+        self.clt("config set identifier = \"foo/a_list\" value=[ \"j\"]", "j")
+        self.clt("config set identifier = \"foo/a_list\" value=[ \"k\" ]", "k")
+
+        # this should raise a TypeError
+        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/a_list\" value=\"a\"")
+        self.assertRaises(isc.cc.data.DataTypeError, self.tool.apply_config_cmd, cmd)
+        
+        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/a_list\" value=[1]")
+        self.assertRaises(isc.cc.data.DataTypeError, self.tool.apply_config_cmd, cmd)
+
+
     
     
+
 class FakeBindCmdInterpreter(bindcmd.BindCmdInterpreter):
 class FakeBindCmdInterpreter(bindcmd.BindCmdInterpreter):
     def __init__(self):
     def __init__(self):
         pass
         pass

+ 88 - 0
src/bin/bindctl/tests/cmdparse_test.py

@@ -0,0 +1,88 @@
+# Copyright (C) 2009  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
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+
+import unittest
+from bindctl import cmdparse
+
+class TestCmdParse(unittest.TestCase):
+
+    def test_remove_unquoted_whitespace(self):
+        self.assertEqual(cmdparse._remove_unquoted_whitespace("a"), "a")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace(" a"), "a")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace("a "), "a")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace(" a "), "a")
+        self.assertNotEqual(cmdparse._remove_unquoted_whitespace("a"), "a ")
+        self.assertNotEqual(cmdparse._remove_unquoted_whitespace(" a"), " a")
+        self.assertNotEqual(cmdparse._remove_unquoted_whitespace("a "), "a ")
+        self.assertNotEqual(cmdparse._remove_unquoted_whitespace(" a "), " a ")
+        self.assertNotEqual(cmdparse._remove_unquoted_whitespace(" a "), "b")
+
+        self.assertEqual(cmdparse._remove_unquoted_whitespace("\"abc\""), "\"abc\"")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace(" \"abc\""), "\"abc\"")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace("\"abc\" "), "\"abc\"")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace(" \"abc\" "), "\"abc\"")
+        
+        self.assertEqual(cmdparse._remove_unquoted_whitespace("\" abc\""), "\" abc\"")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace(" \"a bc\""), "\"a bc\"")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace("\"ab c\" "), "\"ab c\"")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace(" \"abc \" "), "\"abc \"")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace(" \" a b c \" "), "\" a b c \"")
+        
+        self.assertEqual(cmdparse._remove_unquoted_whitespace("a\" abc\"a"), "a\" abc\"a")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace("a \"a bc\"a"), "a\"a bc\"a")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace("a\"ab c\" a"), "a\"ab c\"a")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace("a \"abc \" a"), "a\"abc \"a")
+        self.assertEqual(cmdparse._remove_unquoted_whitespace("a \" a b c \" a"), "a\" a b c \"a")
+
+    # short-hand function to make the set of tests more readable
+    def rws(self, a, b):
+        self.assertEqual(cmdparse._remove_list_and_map_whitespace(a), b)
+
+    def test_remove_list_whitespace(self):
+        self.rws("a", "a")
+        self.rws(" a ", " a ")
+        self.rws(" [a] ", " [a] ")
+        self.rws(" [ a] ", " [a] ")
+        self.rws(" [ a ] ", " [a] ")
+        self.rws(" [ a b c ] ", " [abc] ")
+        self.rws(" [ a \"b c\" ] ", " [a\"b c\"] ")
+        self.rws("a [ a \"b c\" ] a", "a [a\"b c\"] a")
+        self.rws("a] [ a \"b c\" ] a", "a] [a\"b c\"] a")
+        self.rws(" [ a [b c] ] ", " [a[bc]] ")
+        self.rws(" [ a b][ c d ] ", " [ab][cd] ")
+        self.rws(" [ a b] [ c d ] ", " [ab] [cd] ")
+        
+        self.rws("a", "a")
+        self.rws(" a ", " a ")
+        self.rws(" {a} ", " {a} ")
+        self.rws(" { a} ", " {a} ")
+        self.rws(" { a } ", " {a} ")
+        self.rws(" { a b c } ", " {abc} ")
+        self.rws(" { a \"b c\" } ", " {a\"b c\"} ")
+        self.rws("a { a \"b c\" } a", "a {a\"b c\"} a")
+        self.rws("a} { a \"b c\" } a", "a} {a\"b c\"} a")
+        self.rws(" { a {b c} } ", " {a{bc}} ")
+        self.rws(" { a b}{ c d } ", " {ab}{cd} ")
+        self.rws(" { a b} { c d } ", " {ab} {cd} ")
+
+        self.rws(" [ a b]{ c d } ", " [ab]{cd} ")
+        self.rws(" [ a b{ c d }] ", " [ab{cd}] ")
+        self.rws(" [ a b{ \"c d\" }] ", " [ab{\"c d\"}] ")
+        
+
+if __name__== "__main__":
+    unittest.main()
+    

+ 2 - 2
src/lib/config/tests/testdata/spec22.spec

@@ -1,6 +1,6 @@
 {
 {
   "module_spec": {
   "module_spec": {
-    "module_name": "Spec2",
+    "module_name": "Spec22",
     "config_data": [
     "config_data": [
       { "item_name": "value1",
       { "item_name": "value1",
         "item_type": "integer",
         "item_type": "integer",
@@ -81,7 +81,7 @@
       { "item_name": "value9",
       { "item_name": "value9",
         "item_type": "map",
         "item_type": "map",
         "item_optional": false,
         "item_optional": false,
-        "item_default": {},
+        "item_default": { "v91": "def", "v92": {} },
         "map_item_spec": [
         "map_item_spec": [
           { "item_name": "v91",
           { "item_name": "v91",
             "item_type": "string",
             "item_type": "string",

+ 5 - 5
src/lib/python/isc/cc/data.py

@@ -100,8 +100,8 @@ def split_identifier_list_indices(identifier):
 
 
     i = id_str.find('[')
     i = id_str.find('[')
     if i < 0:
     if i < 0:
-        if identifier.find(']') >= 0:
+        if id_str.find(']') >= 0:
-            raise DataTypeError("Bad format in identifier: " + str(identifier))
+            raise DataTypeError("Bad format in identifier (] but no [): " + str(identifier))
         return identifier, None
         return identifier, None
 
 
     # keep the non-index part of that to replace later
     # keep the non-index part of that to replace later
@@ -110,7 +110,7 @@ def split_identifier_list_indices(identifier):
     while i >= 0:
     while i >= 0:
         e = id_str.find(']')
         e = id_str.find(']')
         if e < i + 1:
         if e < i + 1:
-            raise DataTypeError("Bad format in identifier: " + str(identifier))
+            raise DataTypeError("Bad format in identifier (] before [): " + str(identifier))
         try:
         try:
             indices.append(int(id_str[i+1:e]))
             indices.append(int(id_str[i+1:e]))
         except ValueError:
         except ValueError:
@@ -118,9 +118,9 @@ def split_identifier_list_indices(identifier):
         id_str = id_str[e + 1:]
         id_str = id_str[e + 1:]
         i = id_str.find('[')
         i = id_str.find('[')
         if i > 0:
         if i > 0:
-            raise DataTypeError("Bad format in identifier: " + str(identifier))
+            raise DataTypeError("Bad format in identifier ([ within []): " + str(identifier))
     if id.find(']') >= 0 or len(id_str) > 0:
     if id.find(']') >= 0 or len(id_str) > 0:
-        raise DataTypeError("Bad format in identifier: " + str(identifier))
+        raise DataTypeError("Bad format in identifier (extra ]): " + str(identifier))
 
 
     # we replace the final part of the original identifier with
     # we replace the final part of the original identifier with
     # the stripped string
     # the stripped string

+ 18 - 4
src/lib/python/isc/config/ccsession.py

@@ -374,20 +374,34 @@ class UIModuleCCSession(MultiConfigData):
         self._set_current_config(config)
         self._set_current_config(config)
 
 
 
 
-    def add_value(self, identifier, value_str):
+    def add_value(self, identifier, value_str = None):
         """Add a value to a configuration list. Raises a DataTypeError
         """Add a value to a configuration list. Raises a DataTypeError
            if the value does not conform to the list_item_spec field
            if the value does not conform to the list_item_spec field
-           of the module config data specification"""
+           of the module config data specification. If value_str is
+           not given, we add the default as specified by the .spec
+           file."""
         module_spec = self.find_spec_part(identifier)
         module_spec = self.find_spec_part(identifier)
         if (type(module_spec) != dict or "list_item_spec" not in module_spec):
         if (type(module_spec) != dict or "list_item_spec" not in module_spec):
             raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list")
             raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list")
-        value = isc.cc.data.parse_value_str(value_str)
+
         cur_list, status = self.get_value(identifier)
         cur_list, status = self.get_value(identifier)
         if not cur_list:
         if not cur_list:
             cur_list = []
             cur_list = []
+
+        # Hmm. Do we need to check for duplicates?
+        value = None
+        if value_str is not None:
+            value = isc.cc.data.parse_value_str(value_str)
+        else:
+            if "item_default" in module_spec["list_item_spec"]:
+                value = module_spec["list_item_spec"]["item_default"]
+
+        if value is None:
+            raise isc.cc.data.DataNotFoundError("No value given and no default for " + str(identifier))
+            
         if value not in cur_list:
         if value not in cur_list:
             cur_list.append(value)
             cur_list.append(value)
-        self.set_value(identifier, cur_list)
+            self.set_value(identifier, cur_list)
 
 
     def remove_value(self, identifier, value_str):
     def remove_value(self, identifier, value_str):
         """Remove a value from a configuration list. The value string
         """Remove a value from a configuration list. The value string

+ 123 - 58
src/lib/python/isc/config/config_data.py

@@ -121,6 +121,7 @@ def find_spec_part(element, identifier):
         # strip list selector part
         # strip list selector part
         # don't need it for the spec part, so just drop it
         # don't need it for the spec part, so just drop it
         id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
         id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
+        # is this part still needed? (see below)
         if type(cur_el) == dict and 'map_item_spec' in cur_el.keys():
         if type(cur_el) == dict and 'map_item_spec' in cur_el.keys():
             found = False
             found = False
             for cur_el_item in cur_el['map_item_spec']:
             for cur_el_item in cur_el['map_item_spec']:
@@ -128,15 +129,24 @@ def find_spec_part(element, identifier):
                     cur_el = cur_el_item
                     cur_el = cur_el_item
                     found = True
                     found = True
             if not found:
             if not found:
-                raise isc.cc.data.DataNotFoundError(id + " in " + str(cur_el))
+                raise isc.cc.data.DataNotFoundError(id + " not found")
+        elif type(cur_el) == dict and 'list_item_spec' in cur_el.keys():
+            cur_el = cur_el['list_item_spec']
         elif type(cur_el) == list:
         elif type(cur_el) == list:
             found = False
             found = False
             for cur_el_item in cur_el:
             for cur_el_item in cur_el:
                 if cur_el_item['item_name'] == id:
                 if cur_el_item['item_name'] == id:
                     cur_el = cur_el_item
                     cur_el = cur_el_item
+                    # if we need to go further, we may need to 'skip' a step here
+                    # but not if we're done
+                    if id_parts[-1] != id_part and type(cur_el) == dict:
+                        if "map_item_spec" in cur_el:
+                            cur_el = cur_el["map_item_spec"]
+                        elif "list_item_spec" in cur_el:
+                            cur_el = cur_el["list_item_spec"]
                     found = True
                     found = True
             if not found:
             if not found:
-                raise isc.cc.data.DataNotFoundError(id + " in " + str(cur_el))
+                raise isc.cc.data.DataNotFoundError(id + " not found")
         else:
         else:
             raise isc.cc.data.DataNotFoundError("Not a correct config specification")
             raise isc.cc.data.DataNotFoundError("Not a correct config specification")
     return cur_el
     return cur_el
@@ -354,26 +364,63 @@ class MultiConfigData:
            See get_value() for a general way to find a configuration
            See get_value() for a general way to find a configuration
            value
            value
         """
         """
-        if identifier[0] == '/':
-            identifier = identifier[1:]
-        module, sep, id = identifier.partition("/")
         try:
         try:
+            if identifier[0] == '/':
+                identifier = identifier[1:]
+            module, sep, id = identifier.partition("/")
+            # if there is a 'higher-level' list index specified, we need
+            # to check if that list specification has a default that
+            # overrides the more specific default in the final spec item
+            # (ie. list_default = [1, 2, 3], list_item_spec=int, default=0)
+            # def default list[1] should return 2, not 0
+            id_parts = isc.cc.data.split_identifier(id)
+            id_prefix = ""
+            while len(id_parts) > 0:
+                id_part = id_parts.pop(0)
+                item_id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
+                id_list = module + "/" + id_prefix + "/" + item_id
+                id_prefix += "/" + id_part
+                if list_indices is not None:
+                    # there's actually two kinds of default here for
+                    # lists; they can have a default value (like an
+                    # empty list), but their elements can  also have
+                    # default values.
+                    # So if the list item *itself* is a default,
+                    # we need to get the value out of that. If not, we
+                    # need to find the default for the specific element.
+                    list_value, type = self.get_value(id_list) 
+                    list_spec = find_spec_part(self._specifications[module].get_config_spec(), id_prefix)
+                    if type == self.DEFAULT:
+                        if 'item_default' in list_spec:
+                            list_value = list_spec['item_default']
+                            for i in list_indices:
+                                if i < len(list_value):
+                                    list_value = list_value[i]
+                                else:
+                                    # out of range, return None
+                                    return None
+                                
+                            if len(id_parts) > 0:
+                                rest_of_id = "/".join(id_parts)
+                                return isc.cc.data.find(list_value, rest_of_id)
+                            else:
+                                return list_value
+                    else:
+                        # we do have a non-default list, see if our indices
+                        # exist
+                        for i in list_indices:
+                            if i < len(list_value):
+                                list_value = list_value[i]
+                            else:
+                                # out of range, return None
+                                return None
+                    
             spec = find_spec_part(self._specifications[module].get_config_spec(), id)
             spec = find_spec_part(self._specifications[module].get_config_spec(), id)
             if 'item_default' in spec:
             if 'item_default' in spec:
-                id, list_indices = isc.cc.data.split_identifier_list_indices(id)
+                return spec['item_default']
-                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:
             else:
                 return None
                 return None
+
         except isc.cc.data.DataNotFoundError as dnfe:
         except isc.cc.data.DataNotFoundError as dnfe:
             return None
             return None
 
 
@@ -398,13 +445,60 @@ class MultiConfigData:
                 return value, self.DEFAULT
                 return value, self.DEFAULT
         return None, self.NONE
         return None, self.NONE
 
 
-    def get_value_maps(self, identifier = None):
+    def _append_value_item(self, result, spec_part, identifier, all, first = False):
+        # Look at the spec; it is a list of items, or a map containing 'item_name' etc
+        if type(spec_part) == list:
+            for spec_part_element in spec_part:
+                spec_part_element_name = spec_part_element['item_name']
+                self._append_value_item(result, spec_part_element, identifier + "/" + spec_part_element_name, all)
+        elif type(spec_part) == dict:
+            # depending on item type, and the value of argument 'all'
+            # we need to either add an item, or recursively go on
+            # In the case of a list that is empty, we do need to show that
+            item_name = spec_part['item_name']
+            item_type = spec_part['item_type']
+            if item_type == "list" and (all or first):
+                spec_part_list = spec_part['list_item_spec']
+                list_value, status = self.get_value(identifier)
+                if list_value is None:
+                    print("Error: identifier '%s' not found" % identifier)
+                    return
+                if type(list_value) != list:
+                    # the identifier specified a single element
+                    self._append_value_item(result, spec_part_list, identifier, all)
+                else:
+                    list_len = len(list_value)
+                    if len(list_value) == 0 and (all or first):
+                        entry = _create_value_map_entry(identifier,
+                                                        item_type,
+                                                        [], status)
+                        result.append(entry)
+                    else:
+                        for i in range(len(list_value)):
+                            self._append_value_item(result, spec_part_list, "%s[%d]" % (identifier, i), all)
+            elif item_type == "map":
+                # just show the specific contents of a map, we are
+                # almost never interested in just its name
+                spec_part_map = spec_part['map_item_spec']
+                self._append_value_item(result, spec_part_map, identifier, all)
+            else:
+                value, status = self.get_value(identifier)
+                entry = _create_value_map_entry(identifier,
+                                                item_type,
+                                                value, status)
+                result.append(entry)
+        return
+
+
+    def get_value_maps(self, identifier = None, all = False):
         """Returns a list of dicts, containing the following values:
         """Returns a list of dicts, containing the following values:
            name: name of the entry (string)
            name: name of the entry (string)
            type: string containing the type of the value (or 'module')
            type: string containing the type of the value (or 'module')
            value: value of the entry if it is a string, int, double or bool
            value: value of the entry if it is a string, int, double or bool
-           modified: true if the value is a local change
+           modified: true if the value is a local change that has not
-           default: true if the value has been changed
+                     been committed
+           default: true if the value has not been changed (i.e. the
+                    value is the default from the specification)
            TODO: use the consts for those last ones
            TODO: use the consts for those last ones
            Throws DataNotFoundError if the identifier is bad
            Throws DataNotFoundError if the identifier is bad
         """
         """
@@ -412,8 +506,14 @@ class MultiConfigData:
         if not identifier:
         if not identifier:
             # No identifier, so we need the list of current modules
             # No identifier, so we need the list of current modules
             for module in self._specifications.keys():
             for module in self._specifications.keys():
-                entry = _create_value_map_entry(module, 'module', None)
+                if all:
-                result.append(entry)
+                    spec = self.get_module_spec(module)
+                    if spec:
+                        spec_part = spec.get_config_spec()
+                        self._append_value_item(result, spec_part, module, all, True)
+                else:
+                    entry = _create_value_map_entry(module, 'module', None)
+                    result.append(entry)
         else:
         else:
             if identifier[0] == '/':
             if identifier[0] == '/':
                 identifier = identifier[1:]
                 identifier = identifier[1:]
@@ -421,42 +521,7 @@ class MultiConfigData:
             spec = self.get_module_spec(module)
             spec = self.get_module_spec(module)
             if spec:
             if spec:
                 spec_part = find_spec_part(spec.get_config_spec(), id)
                 spec_part = find_spec_part(spec.get_config_spec(), id)
-                if type(spec_part) == list:
+                self._append_value_item(result, spec_part, identifier, all, True)
-                    # list of items to show
-                    for item in spec_part:
-                        value, status = self.get_value("/" + identifier\
-                                              + "/" + item['item_name'])
-                        entry = _create_value_map_entry(item['item_name'],
-                                                        item['item_type'],
-                                                        value, status)
-                        result.append(entry)
-                elif type(spec_part) == dict:
-                    # Sub-specification
-                    item = spec_part
-                    if item['item_type'] == 'list':
-                        li_spec = item['list_item_spec']
-                        value, status =  self.get_value("/" + identifier)
-                        if type(value) == list:
-                            for list_value in value:
-                                result_part2 = _create_value_map_entry(
-                                                   li_spec['item_name'],
-                                                   li_spec['item_type'],
-                                                   list_value)
-                                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:
-                        value, status = self.get_value("/" + identifier)
-                        if value is not None:
-                            entry = _create_value_map_entry(
-                                        item['item_name'],
-                                        item['item_type'],
-                                        value, status)
-                            result.append(entry)
         return result
         return result
 
 
     def set_value(self, identifier, value):
     def set_value(self, identifier, value):

+ 55 - 21
src/lib/python/isc/config/tests/config_data_test.py

@@ -358,6 +358,8 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(1, value)
         self.assertEqual(1, value)
         value = self.mcd.get_default_value("Spec2/item5[0]")
         value = self.mcd.get_default_value("Spec2/item5[0]")
         self.assertEqual('a', value)
         self.assertEqual('a', value)
+        value = self.mcd.get_default_value("Spec2/item5[1]")
+        self.assertEqual('b', value)
         value = self.mcd.get_default_value("Spec2/item5[5]")
         value = self.mcd.get_default_value("Spec2/item5[5]")
         self.assertEqual(None, value)
         self.assertEqual(None, value)
         value = self.mcd.get_default_value("Spec2/item5[0][1]")
         value = self.mcd.get_default_value("Spec2/item5[0][1]")
@@ -392,6 +394,10 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(None, value)
         self.assertEqual(None, value)
         self.assertEqual(MultiConfigData.NONE, status)
         self.assertEqual(MultiConfigData.NONE, status)
 
 
+        value, status = self.mcd.get_value("Spec2/item5")
+        self.assertEqual(['a', 'b'], value)
+        self.assertEqual(MultiConfigData.DEFAULT, status)
+
         value, status = self.mcd.get_value("Spec2/item5[0]")
         value, status = self.mcd.get_value("Spec2/item5[0]")
         self.assertEqual("a", value)
         self.assertEqual("a", value)
         self.assertEqual(MultiConfigData.DEFAULT, status)
         self.assertEqual(MultiConfigData.DEFAULT, status)
@@ -400,6 +406,11 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(None, value)
         self.assertEqual(None, value)
         self.assertEqual(MultiConfigData.NONE, status)
         self.assertEqual(MultiConfigData.NONE, status)
 
 
+        value, status = self.mcd.get_value("Spec2/item5[1]")
+        self.assertEqual("b", value)
+        self.assertEqual(MultiConfigData.DEFAULT, status)
+
+
 
 
     def test_get_value_maps(self):
     def test_get_value_maps(self):
         maps = self.mcd.get_value_maps()
         maps = self.mcd.get_value_maps()
@@ -423,32 +434,34 @@ class TestMultiConfigData(unittest.TestCase):
         self.mcd._set_current_config({ "Spec2": { "item1": 2 } })
         self.mcd._set_current_config({ "Spec2": { "item1": 2 } })
         self.mcd.set_value("Spec2/item3", False)
         self.mcd.set_value("Spec2/item3", False)
         maps = self.mcd.get_value_maps("/Spec2")
         maps = self.mcd.get_value_maps("/Spec2")
-        self.assertEqual([{'default': False, 'type': 'integer', 'name': 'item1', 'value': 2, 'modified': False},
+        self.assertEqual([{'default': False, 'type': 'integer', 'name': 'Spec2/item1', 'value': 2, 'modified': False},
-                          {'default': True, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
+                          {'default': True, 'type': 'real', 'name': 'Spec2/item2', 'value': 1.1, 'modified': False},
-                          {'default': False, 'type': 'boolean', 'name': 'item3', 'value': False, 'modified': True},
+                          {'default': False, 'type': 'boolean', 'name': 'Spec2/item3', 'value': False, 'modified': True},
-                          {'default': True, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
+                          {'default': True, 'type': 'string', 'name': 'Spec2/item4', 'value': 'test', 'modified': False},
-                          {'default': True, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
+                          {'default': True, 'type': 'list', 'name': 'Spec2/item5', 'value': ['a', 'b'], 'modified': False},
-                          {'default': True, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
+                          {'default': True, 'type': 'string', 'name': 'Spec2/item6/value1', 'value': 'default', 'modified': False},
+                          {'default': False, 'type': 'integer', 'name': 'Spec2/item6/value2', 'value': None, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("Spec2")
         maps = self.mcd.get_value_maps("Spec2")
-        self.assertEqual([{'default': False, 'type': 'integer', 'name': 'item1', 'value': 2, 'modified': False},
+        self.assertEqual([{'default': False, 'type': 'integer', 'name': 'Spec2/item1', 'value': 2, 'modified': False},
-                          {'default': True, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
+                          {'default': True, 'type': 'real', 'name': 'Spec2/item2', 'value': 1.1, 'modified': False},
-                          {'default': False, 'type': 'boolean', 'name': 'item3', 'value': False, 'modified': True},
+                          {'default': False, 'type': 'boolean', 'name': 'Spec2/item3', 'value': False, 'modified': True},
-                          {'default': True, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
+                          {'default': True, 'type': 'string', 'name': 'Spec2/item4', 'value': 'test', 'modified': False},
-                          {'default': True, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
+                          {'default': True, 'type': 'list', 'name': 'Spec2/item5', 'value': ['a', 'b'], 'modified': False},
-                          {'default': True, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
+                          {'default': True, 'type': 'string', 'name': 'Spec2/item6/value1', 'value': 'default', 'modified': False},
+                          {'default': False, 'type': 'integer', 'name': 'Spec2/item6/value2', 'value': None, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item5")
         maps = self.mcd.get_value_maps("/Spec2/item5")
-        self.assertEqual([{'default': False, 'type': 'string', 'name': 'list_element', 'value': 'a', 'modified': False},
+        self.assertEqual([{'default': True, 'type': 'string', 'name': 'Spec2/item5[0]', 'value': 'a', 'modified': False},
-                          {'default': False, 'type': 'string', 'name': 'list_element', 'value': 'b', 'modified': False}], maps)
+                          {'default': True, 'type': 'string', 'name': 'Spec2/item5[1]', 'value': 'b', 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item5[0]")
         maps = self.mcd.get_value_maps("/Spec2/item5[0]")
-        self.assertEqual([{'default': True, 'modified': False, 'name': 'list_element', 'type': 'string', 'value': 'a'}], maps)
+        self.assertEqual([{'default': True, 'modified': False, 'name': 'Spec2/item5[0]', 'type': 'string', 'value': 'a'}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item1")
         maps = self.mcd.get_value_maps("/Spec2/item1")
-        self.assertEqual([{'default': False, 'type': 'integer', 'name': 'item1', 'value': 2, 'modified': False}], maps)
+        self.assertEqual([{'default': False, 'type': 'integer', 'name': 'Spec2/item1', 'value': 2, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item2")
         maps = self.mcd.get_value_maps("/Spec2/item2")
-        self.assertEqual([{'default': True, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False}], maps)
+        self.assertEqual([{'default': True, 'type': 'real', 'name': 'Spec2/item2', 'value': 1.1, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item3")
         maps = self.mcd.get_value_maps("/Spec2/item3")
-        self.assertEqual([{'default': False, 'type': 'boolean', 'name': 'item3', 'value': False, 'modified': True}], maps)
+        self.assertEqual([{'default': False, 'type': 'boolean', 'name': 'Spec2/item3', 'value': False, 'modified': True}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item4")
         maps = self.mcd.get_value_maps("/Spec2/item4")
-        self.assertEqual([{'default': True, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False}], maps)
+        self.assertEqual([{'default': True, 'type': 'string', 'name': 'Spec2/item4', 'value': 'test', 'modified': False}], maps)
 
 
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec24.spec")
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec24.spec")
         self.mcd.set_specification(module_spec)
         self.mcd.set_specification(module_spec)
@@ -456,9 +469,30 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual([], maps)
         self.assertEqual([], maps)
         self.mcd._set_current_config({ "Spec24": { "item": [] } })
         self.mcd._set_current_config({ "Spec24": { "item": [] } })
         maps = self.mcd.get_value_maps("/Spec24/item")
         maps = self.mcd.get_value_maps("/Spec24/item")
-        self.assertEqual([], maps)
+        self.assertEqual([{'default': False, 'modified': False, 'name': 'Spec24/item', 'type': 'list', 'value': []}], maps)
-
 
 
+        module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec22.spec")
+        self.mcd.set_specification(module_spec)
+        expected = [{'default': True,
+                     'modified': False,
+                     'name': 'Spec22/value9/v91',
+                     'type': 'string',
+                     'value': 'def'},
+                    {'default': True,
+                     'modified': False,
+                     'name': 'Spec22/value9/v92/v92a',
+                     'type': 'string',
+                     'value': 'Hello'
+                    },
+                    {'default': True,
+                     'modified': False,
+                     'name': 'Spec22/value9/v92/v92b',
+                     'type': 'integer',
+                     'value': 47806
+                    }
+                   ]
+        maps = self.mcd.get_value_maps("/Spec22/value9")
+        self.assertEqual(expected, maps)
 
 
     def test_set_value(self):
     def test_set_value(self):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")