Finish implementing MultiStrOpt

Fixes bug 955308

Previously only multiple string options from the CLI were supported.
This change adds support for config files too and merges the results
from both CLI and config files.

Change-Id: I642408c03ed295fac050105fd4380940e876f228

2 files changed, 232 insertions, 27 deletions

diff --git a/openstack/common/cfg.py b/openstack/common/cfg.py
index 1005b2f..ff81a5c 100644
--- a/openstack/common/cfg.py
+++ b/openstack/common/cfg.py
@@ -101,9 +101,9 @@ The config manager has a single CLI option defined by default, --config-file::
             ...
             self.register_cli_opt(self.config_file_opt)
 
-Option values are parsed from any supplied config files using SafeConfigParser.
-If none are specified, a default set is used e.g. glance-api.conf and
-glance-common.conf::
+Option values are parsed from any supplied config files using
+openstack.common.iniparser. If none are specified, a default set is used
+e.g. glance-api.conf and glance-common.conf::
 
     glance-api.conf:
       [DEFAULT]
@@ -210,13 +210,14 @@ i.e. argument parsing is stopped at the first non-option argument.
 """
 
 import collections
-import ConfigParser
 import copy
 import optparse
 import os
 import string
 import sys
 
+import iniparser
+
 
 class Error(Exception):
     """Base class for cfg exceptions."""
@@ -398,6 +399,7 @@ class Opt(object):
       help:
         an string explaining how the options value is used
     """
+    multi = False
 
     def __init__(self, name, dest=None, short=None,
                  default=None, metavar=None, help=None):
@@ -424,7 +426,7 @@ class Opt(object):
         self.help = help
 
     def _get_from_config_parser(self, cparser, section):
-        """Retrieves the option value from a ConfigParser object.
+        """Retrieves the option value from a MultiConfigParser object.
 
         This is the method ConfigOpts uses to look up the option value from
         config files. Most opt types override this method in order to perform
@@ -433,7 +435,7 @@ class Opt(object):
         :param cparser: a ConfigParser object
         :param section: a section name
         """
-        return cparser.get(section, self.dest, raw=True)
+        return cparser.get(section, self.dest)
 
     def _add_to_cli(self, parser, group=None):
         """Makes the option available in the command line interface.
@@ -535,9 +537,19 @@ class BoolOpt(Opt):
     1/0, yes/no, true/false or on/off.
     """
 
+    _boolean_states = {'1': True, 'yes': True, 'true': True, 'on': True,
+                       '0': False, 'no': False, 'false': False, 'off': False}
+
     def _get_from_config_parser(self, cparser, section):
         """Retrieve the opt value as a boolean from ConfigParser."""
-        return cparser.getboolean(section, self.dest)
+        def convert_bool(v):
+            value = self._boolean_states.get(v.lower())
+            if value is None:
+                raise ValueError('Unexpected boolean value %r' % v)
+
+            return value
+
+        return [convert_bool(v) for v in cparser.get(section, self.dest)]
 
     def _add_to_cli(self, parser, group=None):
         """Extends the base class method to add the --nooptname option."""
@@ -564,7 +576,7 @@ class IntOpt(Opt):
 
     def _get_from_config_parser(self, cparser, section):
         """Retrieve the opt value as a integer from ConfigParser."""
-        return cparser.getint(section, self.dest)
+        return [int(v) for v in cparser.get(section, self.dest)]
 
     def _get_optparse_kwargs(self, group, **kwargs):
         """Extends the base optparse keyword dict for integer options."""
@@ -578,7 +590,7 @@ class FloatOpt(Opt):
 
     def _get_from_config_parser(self, cparser, section):
         """Retrieve the opt value as a float from ConfigParser."""
-        return cparser.getfloat(section, self.dest)
+        return [float(v) for v in cparser.get(section, self.dest)]
 
     def _get_optparse_kwargs(self, group, **kwargs):
         """Extends the base optparse keyword dict for float options."""
@@ -595,7 +607,7 @@ class ListOpt(Opt):
 
     def _get_from_config_parser(self, cparser, section):
         """Retrieve the opt value as a list from ConfigParser."""
-        return cparser.get(section, self.dest).split(',')
+        return [v.split(',') for v in cparser.get(section, self.dest)]
 
     def _get_optparse_kwargs(self, group, **kwargs):
         """Extends the base optparse keyword dict for list options."""
@@ -617,14 +629,7 @@ class MultiStrOpt(Opt):
     Multistr opt values are string opts which may be specified multiple times.
     The opt value is a list containing all the string values specified.
     """
-
-    def _get_from_config_parser(self, cparser, section):
-        """Retrieve the opt value as a multistr from ConfigParser."""
-        # FIXME(markmc): values spread across the CLI and multiple
-        #                config files should be appended
-        value = super(MultiStrOpt, self)._get_from_config_parser(cparser,
-                                                                 section)
-        return value if value is None else [value]
+    multi = True
 
     def _get_optparse_kwargs(self, group, **kwargs):
         """Extends the base optparse keyword dict for multi str options."""
@@ -691,6 +696,69 @@ class OptGroup(object):
         return self._optparse_group
 
 
+class ParseError(iniparser.ParseError):
+    def __init__(self, msg, lineno, line, filename):
+        super(ParseError, self).__init__(msg, lineno, line)
+        self.filename = filename
+
+    def __str__(self):
+        return 'at %s:%d, %s: %r' % (self.filename, self.lineno,
+                                     self.msg, self.line)
+
+
+class ConfigParser(iniparser.BaseParser):
+    def __init__(self, filename, sections):
+        super(ConfigParser, self).__init__()
+        self.filename = filename
+        self.sections = sections
+        self.section = None
+
+    def parse(self):
+        with open(self.filename) as f:
+            return super(ConfigParser, self).parse(f)
+
+    def new_section(self, section):
+        self.section = section
+        self.sections.setdefault(self.section, {})
+
+    def assignment(self, key, value):
+        if not self.section:
+            raise self.error_no_section()
+
+        self.sections[self.section].setdefault(key, [])
+        self.sections[self.section][key].append('\n'.join(value))
+
+    def parse_exc(self, msg, lineno, line=None):
+        return ParseError(msg, lineno, line, self.filename)
+
+    def error_no_section(self):
+        return self.parse_exc('Section must be started before assignment',
+                              self.lineno)
+
+
+class MultiConfigParser(object):
+    def __init__(self):
+        self.sections = {}
+
+    def read(self, config_files):
+        read_ok = []
+
+        for filename in config_files:
+            parser = ConfigParser(filename, self.sections)
+
+            try:
+                parser.parse()
+            except IOError:
+                continue
+
+            read_ok.append(filename)
+
+        return read_ok
+
+    def get(self, section, name):
+        return self.sections[section][name]
+
+
 class ConfigOpts(collections.Mapping):
 
     """
@@ -986,20 +1054,31 @@ class ConfigOpts(collections.Mapping):
         if override is not None:
             return override
 
+        values = []
         if self._cparser is not None:
             section = group.name if group is not None else 'DEFAULT'
             try:
-                return opt._get_from_config_parser(self._cparser, section)
-            except (ConfigParser.NoOptionError,
-                    ConfigParser.NoSectionError):
+                value = opt._get_from_config_parser(self._cparser, section)
+            except KeyError:
                 pass
-            except ValueError, ve:
+            except ValueError as ve:
                 raise ConfigFileValueError(str(ve))
+            else:
+                if not opt.multi:
+                    # No need to continue since the last value wins
+                    return value[-1]
+                values.extend(value)
 
         name = name if group is None else group.name + '_' + name
-        value = self._cli_values.get(name, None)
+        value = self._cli_values.get(name)
         if value is not None:
-            return value
+            if not opt.multi:
+                return value
+
+            return value + values
+
+        if values:
+            return values
 
         if default is not None:
             return default
@@ -1069,12 +1148,12 @@ class ConfigOpts(collections.Mapping):
 
         :raises: ConfigFilesNotFoundError, ConfigFileParseError
         """
-        self._cparser = ConfigParser.SafeConfigParser()
+        self._cparser = MultiConfigParser()
 
         try:
             read_ok = self._cparser.read(config_files)
-        except ConfigParser.ParsingError, cpe:
-            raise ConfigFileParseError(cpe.filename, cpe.message)
+        except iniparser.ParseError as pe:
+            raise ConfigFileParseError(pe.filename, str(pe))
 
         if read_ok != config_files:
             not_read_ok = filter(lambda f: f not in read_ok, config_files)
diff --git a/openstack/common/iniparser.py b/openstack/common/iniparser.py
new file mode 100644
index 0000000..53ca023
--- /dev/null
+++ b/openstack/common/iniparser.py
@@ -0,0 +1,126 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2012 OpenStack LLC.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+
+class ParseError(Exception):
+    def __init__(self, message, lineno, line):
+        self.msg = message
+        self.line = line
+        self.lineno = lineno
+
+    def __str__(self):
+        return 'at line %d, %s: %r' % (self.lineno, self.msg, self.line)
+
+
+class BaseParser(object):
+    lineno = 0
+    parse_exc = ParseError
+
+    def _assignment(self, key, value):
+        self.assignment(key, value)
+        return None, []
+
+    def _get_section(self, line):
+        if line[-1] != ']':
+            return self.error_no_section_end_bracket(line)
+        if len(line) <= 2:
+            return self.error_no_section_name(line)
+
+        return line[1:-1]
+
+    def _split_key_value(self, line):
+        colon = line.find(':')
+        equal = line.find('=')
+        if colon < 0 and equal < 0:
+            return self.error_invalid_assignment(line)
+
+        if colon < 0 or (equal >= 0 and equal < colon):
+            key, value = line[:equal], line[equal + 1:]
+        else:
+            key, value = line[:colon], line[colon + 1:]
+
+        return key.strip(), [value.strip()]
+
+    def parse(self, lineiter):
+        key = None
+        value = []
+
+        for line in lineiter:
+            self.lineno += 1
+
+            line = line.rstrip()
+            if not line:
+                # Blank line, ends multi-line values
+                if key:
+                    key, value = self._assignment(key, value)
+                continue
+            elif line[0] in (' ', '\t'):
+                # Continuation of previous assignment
+                if key is None:
+                    self.error_unexpected_continuation(line)
+                else:
+                    value.append(line.lstrip())
+                continue
+
+            if key:
+                # Flush previous assignment, if any
+                key, value = self._assignment(key, value)
+
+            if line[0] == '[':
+                # Section start
+                section = self._get_section(line)
+                if section:
+                    self.new_section(section)
+            elif line[0] in '#;':
+                self.comment(line[1:].lstrip())
+            else:
+                key, value = self._split_key_value(line)
+                if not key:
+                    return self.error_empty_key(line)
+
+        if key:
+            # Flush previous assignment, if any
+            self._assignment(key, value)
+
+    def assignment(self, key, value):
+        """Called when a full assignment is parsed"""
+        raise NotImplementedError()
+
+    def new_section(self, section):
+        """Called when a new section is started"""
+        raise NotImplementedError()
+
+    def comment(self, comment):
+        """Called when a comment is parsed"""
+        pass
+
+    def error_invalid_assignment(self, line):
+        raise self.parse_exc("No ':' or '=' found in assignment",
+                             self.lineno, line)
+
+    def error_empty_key(self, line):
+        raise self.parse_exc('Key cannot be empty', self.lineno, line)
+
+    def error_unexpected_continuation(self, line):
+        raise self.parse_exc('Unexpected continuation line',
+                             self.lineno, line)
+
+    def error_no_section_end_bracket(self, line):
+        raise self.parse_exc('Invalid section (must end with ])',
+                             self.lineno, line)
+
+    def error_no_section_name(self, line):
+        raise self.parse_exc('Empty section name', self.lineno, line)