Options: Fixed non-path database name parsing

The database name is sometimes a path (SQLite) and sometimes not
(MySQL, PostgreSQL). This introduces a new Option type,
RepositoryMacroOption, that expands <repository> macros without
canonicalizing the path, so SQLite users can use <repository> in their
settings but MySQL users' database name settings will not be destroyed
by path canonicalization.

The unfortunate downside is that SQLite users can't use ~ in their
database name.

3 files changed, 54 insertions, 27 deletions

diff --git a/src/lib/Bcfg2/DBSettings.py b/src/lib/Bcfg2/DBSettings.py
index a8f88d3d4..982a299c0 100644
--- a/src/lib/Bcfg2/DBSettings.py
+++ b/src/lib/Bcfg2/DBSettings.py
@@ -211,7 +211,7 @@ class _OptionContainer(object):
         Bcfg2.Options.Option(
             cf=('database', 'engine'), default='sqlite3',
             help='Database engine', dest='db_engine'),
-        Bcfg2.Options.PathOption(
+        Bcfg2.Options.RepositoryMacroOption(
             cf=('database', 'name'), default='<repository>/etc/bcfg2.sqlite',
             help="Database name", dest="db_name"),
         Bcfg2.Options.Option(
diff --git a/src/lib/Bcfg2/Options/Options.py b/src/lib/Bcfg2/Options/Options.py
index f1a201f1e..752e01b4e 100644
--- a/src/lib/Bcfg2/Options/Options.py
+++ b/src/lib/Bcfg2/Options/Options.py
@@ -14,8 +14,8 @@ from Bcfg2.Options import Types
 from Bcfg2.Compat import ConfigParser
 
 
-__all__ = ["Option", "BooleanOption", "PathOption", "PositionalArgument",
-           "_debug"]
+__all__ = ["Option", "BooleanOption", "RepositoryMacroOption", "PathOption",
+           "PositionalArgument", "_debug"]
 
 unit_test = False  # pylint: disable=C0103
 
@@ -326,24 +326,13 @@ class Option(object):
                    (self, parser))
 
 
-class PathOption(Option):
-    """Shortcut for options that expect a path argument.
-
-    Uses :meth:`Bcfg2.Options.Types.path` to transform the argument
-    into a canonical path. The type of a path option can also be
-    overridden to return a file-like object. For example:
-
-    .. code-block:: python
-
-        options = [
-            Bcfg2.Options.PathOption(
-                "--input", type=argparse.FileType('r'),
-                help="The input file")]
+class RepositoryMacroOption(Option):
+    """Option that does translation of ``<repository>`` macros.
 
-    PathOptions also do translation of ``<repository>`` macros on the
-    fly. It's done this way instead of just fixing up all values at
-    the end of parsing because macro expansion needs to be done before
-    path canonicalization and other stuff.
+    Macro translation is done on the fly instead of just fixing up all
+    values at the end of parsing because macro expansion needs to be
+    done before path canonicalization for
+    :class:`Bcfg2.Options.Options.PathOption`.
     """
     repository = None
 
@@ -356,8 +345,8 @@ class PathOption(Option):
     def early_parsing_hook(self, early_opts):
         if hasattr(early_opts, "repository"):
             if self.__class__.repository is None:
-                _debug("Setting repository to %s for PathOptions" %
-                       early_opts.repository)
+                _debug("Setting repository to %s for %s" %
+                       (early_opts.repository, self.__class__.__name__))
                 self.__class__.repository = early_opts.repository
             else:
                 _debug("Repository is already set for %s" % self.__class__)
@@ -371,15 +360,45 @@ class PathOption(Option):
 
     default = property(_get_default, Option._set_default)
 
+    def transform_value(self, value):
+        """transform the value after macro expansion.
+
+         this can be overridden to further transform the value set by
+        the user *after* macros are expanded, but before the user's
+        ``type`` function is applied. principally exists for
+        PathOption to canonicalize the path.
+        """
+        return value
+
     def _type(self, value):
         """Type function that fixes up <repository> macros."""
         if self.__class__.repository is None:
             return value
         else:
-            return self._original_type(Types.path(
+            return self._original_type(self.transform_value(
                 value.replace("<repository>", self.__class__.repository)))
 
 
+class PathOption(RepositoryMacroOption):
+    """Shortcut for options that expect a path argument.
+
+    Uses :meth:`Bcfg2.Options.Types.path` to transform the argument
+    into a canonical path. The type of a path option can also be
+    overridden to return a file-like object. For example:
+
+    .. code-block:: python
+
+        options = [
+            Bcfg2.Options.PathOption(
+                "--input", type=argparse.FileType('r'),
+                help="The input file")]
+
+    PathOptions also do translation of ``<repository>`` macros.
+    """
+    def transform_value(self, value):
+        return Types.path(value)
+
+
 class _BooleanOptionAction(argparse.Action):
     """BooleanOptionAction sets a boolean value.
 
diff --git a/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py b/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py
index a3190f2ca..a2dc8ffe2 100644
--- a/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py
+++ b/testsuite/Testsrc/Testlib/TestOptions/TestOptions.py
@@ -7,8 +7,9 @@ import tempfile
 import mock
 
 from Bcfg2.Compat import ConfigParser
-from Bcfg2.Options import Option, PathOption, BooleanOption, Parser, \
-    PositionalArgument, OptionParserException, Common, new_parser, get_parser
+from Bcfg2.Options import Option, PathOption, RepositoryMacroOption, \
+    BooleanOption, Parser, PositionalArgument, OptionParserException, \
+    Common, new_parser, get_parser
 from testsuite.Testsrc.Testlib.TestOptions import OptionTestCase, \
     make_config, clean_environment
 
@@ -382,16 +383,21 @@ class TestBasicOptions(OptionTestCase):
             parser.add_options,
             [Option(cf=("test", "option"))])
 
-    @make_config({"test": {"test_path": "<repository>/test"}})
+    @make_config({"test": {"test_path": "<repository>/test",
+                           "test_macro": "<repository>"}})
     def test_repository_macro(self, config_file):
         """fix up <repository> macros."""
         result = argparse.Namespace()
         parser = Parser(namespace=result)
         parser.add_options([PathOption("--test1"),
-                            PathOption("--test2"),
+                            RepositoryMacroOption("--test2"),
                             PathOption(cf=("test", "test_path")),
                             PathOption(cf=("test", "test_path_default"),
                                        default="<repository>/test/default"),
+                            RepositoryMacroOption(cf=("test", "test_macro")),
+                            RepositoryMacroOption(
+                                cf=("test", "test_macro_default"),
+                                default="<repository>"),
                             Common.repository])
         parser.parse(["-C", config_file, "-Q", "/foo/bar",
                       "--test1", "<repository>/test1",
@@ -399,6 +405,8 @@ class TestBasicOptions(OptionTestCase):
         self.assertEqual(result.repository, "/foo/bar")
         self.assertEqual(result.test1, "/foo/bar/test1")
         self.assertEqual(result.test2, "/foo/bar/foo/bar")
+        self.assertEqual(result.test_macro, "/foo/bar")
+        self.assertEqual(result.test_macro_default, "/foo/bar")
         self.assertEqual(result.test_path, "/foo/bar/test")
         self.assertEqual(result.test_path_default, "/foo/bar/test/default")