Merge "Finish implementing MultiStrOpt"

4 files changed, 359 insertions, 34 deletions

diff --git a/openstack/common/cfg.py b/openstack/common/cfg.py
index 7b729c3..7adf3b4 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]
@@ -220,13 +220,14 @@ log files:
 """
 
 import collections
-import ConfigParser
 import copy
 import optparse
 import os
 import string
 import sys
 
+import iniparser
+
 
 class Error(Exception):
     """Base class for cfg exceptions."""
@@ -408,6 +409,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, secret=False):
@@ -436,7 +438,7 @@ class Opt(object):
         self.secret = secret
 
     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
@@ -445,7 +447,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.
@@ -547,9 +549,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."""
@@ -576,7 +588,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."""
@@ -590,7 +602,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."""
@@ -607,7 +619,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."""
@@ -629,14 +641,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."""
@@ -703,6 +708,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):
 
     """
@@ -1005,20 +1073,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
@@ -1088,12 +1167,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)
diff --git a/tests/unit/test_cfg.py b/tests/unit/test_cfg.py
index 9e1e0e8..4e8ec21 100644
--- a/tests/unit/test_cfg.py
+++ b/tests/unit/test_cfg.py
@@ -445,24 +445,23 @@ class ConfigFileOptsTestCase(BaseTestCase):
         self.assertEquals(self.conf.foo, ['bar'])
 
     def test_conf_file_multistr_values_append(self):
-        self.conf.register_cli_opt(ListOpt('foo'))
+        self.conf.register_cli_opt(MultiStrOpt('foo'))
 
         paths = self.create_tempfiles([('1.conf',
                                         '[DEFAULT]\n'
-                                        'foo = bar\n'),
+                                        'foo = bar1\n'),
                                        ('2.conf',
                                         '[DEFAULT]\n'
-                                        'foo = bar\n')])
+                                        'foo = bar2\n'
+                                        'foo = bar3\n')])
 
-        self.conf(['--foo', 'bar',
+        self.conf(['--foo', 'bar0',
                    '--config-file', paths[0],
                    '--config-file', paths[1]])
 
         self.assertTrue(hasattr(self.conf, 'foo'))
 
-        # FIXME(markmc): values spread across the CLI and multiple
-        #                config files should be appended
-        # self.assertEquals(self.conf.foo, ['bar', 'bar', 'bar'])
+        self.assertEquals(self.conf.foo, ['bar0', 'bar1', 'bar2', 'bar3'])
 
     def test_conf_file_multiple_opts(self):
         self.conf.register_opts([StrOpt('foo'), StrOpt('bar')])
@@ -989,3 +988,13 @@ class CommonOptsTestCase(BaseTestCase):
                           CommonConfigOpts.DEFAULT_LOG_DATE_FORMAT)
 
         self.assertEquals(self.conf.use_syslog, False)
+
+
+class ConfigParserTestCase(unittest.TestCase):
+    def test_no_section(self):
+        with tempfile.NamedTemporaryFile() as tmpfile:
+            tmpfile.write('foo = bar')
+            tmpfile.flush()
+
+            parser = ConfigParser(tmpfile.name, {})
+            self.assertRaises(ParseError, parser.parse)
diff --git a/tests/unit/test_iniparser.py b/tests/unit/test_iniparser.py
new file mode 100644
index 0000000..5b3a0de
--- /dev/null
+++ b/tests/unit/test_iniparser.py
@@ -0,0 +1,111 @@
+# 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.
+
+import unittest
+
+from openstack.common import iniparser
+
+
+class TestParser(iniparser.BaseParser):
+    comment_called = False
+    values = None
+    section = ''
+
+    def __init__(self):
+        self.values = {}
+
+    def assignment(self, key, value):
+        self.values.setdefault(self.section, {})
+        self.values[self.section][key] = value
+
+    def new_section(self, section):
+        self.section = section
+
+    def comment(self, section):
+        self.comment_called = True
+
+
+class BaseParserTestCase(unittest.TestCase):
+    def setUp(self):
+        self.parser = iniparser.BaseParser()
+
+    def _assertParseError(self, *lines):
+        self.assertRaises(iniparser.ParseError, self.parser.parse, lines)
+
+    def test_invalid_assignment(self):
+        self._assertParseError("foo - bar")
+
+    def test_empty_key(self):
+        self._assertParseError(": bar")
+
+    def test_unexpected_continuation(self):
+        self._assertParseError("   baz")
+
+    def test_invalid_section(self):
+        self._assertParseError("[section")
+
+    def test_no_section_name(self):
+        self._assertParseError("[]")
+
+
+class ParserTestCase(unittest.TestCase):
+    def setUp(self):
+        self.parser = TestParser()
+
+    def test_blank_line(self):
+        lines = [""]
+        self.parser.parse(lines)
+        self.assertEquals(self.parser.values, {})
+
+    def test_assignment_equal(self):
+        lines = ["foo = bar"]
+        self.parser.parse(lines)
+        self.assertEquals(self.parser.values, {'': {'foo': ['bar']}})
+
+    def test_assignment_colon(self):
+        lines = ["foo: bar"]
+        self.parser.parse(lines)
+        self.assertEquals(self.parser.values, {'': {'foo': ['bar']}})
+
+    def test_assignment_multiline(self):
+        lines = ["foo = bar0", "  bar1"]
+        self.parser.parse(lines)
+        self.assertEquals(self.parser.values, {'': {'foo': ['bar0', 'bar1']}})
+
+    def test_assignment_multline_empty(self):
+        lines = ["foo = bar0", "", "  bar1"]
+        self.assertRaises(iniparser.ParseError, self.parser.parse, lines)
+
+    def test_section_assignment(self):
+        lines = ["[test]", "foo = bar"]
+        self.parser.parse(lines)
+        self.assertEquals(self.parser.values, {'test': {'foo': ['bar']}})
+
+    def test_new_section(self):
+        lines = ["[foo]"]
+        self.parser.parse(lines)
+        self.assertEquals(self.parser.section, 'foo')
+
+    def test_comment(self):
+        lines = ["# foobar"]
+        self.parser.parse(lines)
+        self.assertTrue(self.parser.comment_called)
+
+
+class ExceptionTestCase(unittest.TestCase):
+    def test_parseerror(self):
+        exc = iniparser.ParseError('test', 42, 'example')
+        self.assertEquals(str(exc), "at line 42, test: 'example'")