Allow set_default and set_override to use None

The current implementation interprets set_default('foo', None) and
set_override('foo', None) as 'clear the existing default or override',
which makes it impossible to override a value with None.

This patch adds support for overriding with a None value by adding
a special internal class. set_override('foo', None) will now override
the existing value with None. This is a slight change to the existing
behavior, so this patch adds two calls for the old functionality of
clearing defaults and overrides. Example syntax for the new calls
are shown below:

conf.clear_default('foo')
conf.clear_override('foo')

The patch also updates the tests to reflect the change in functionality
and adds new tests to verify the new functionality.

Fixes bug 1035478

Change-Id: Iee5e20e44da9bef6b86e0483ab0b48b625fe503c

2 files changed, 68 insertions, 4 deletions

diff --git a/openstack/common/cfg.py b/openstack/common/cfg.py
index b42c30e..a56f957 100644
--- a/openstack/common/cfg.py
+++ b/openstack/common/cfg.py
@@ -1175,6 +1175,9 @@ class ConfigOpts(collections.Mapping):
         __import__(module_str)
         self._get_opt_info(name, group)
 
+    class _NoneValue(object):
+        pass
+
     @__clear_cache
     def set_override(self, name, override, group=None):
         """Override an opt value.
@@ -1187,6 +1190,8 @@ class ConfigOpts(collections.Mapping):
         :param group: an option OptGroup object or group name
         :raises: NoSuchOptError, NoSuchGroupError
         """
+        if override is None:
+            override = self._NoneValue()
         opt_info = self._get_opt_info(name, group)
         opt_info['override'] = override
 
@@ -1202,9 +1207,38 @@ class ConfigOpts(collections.Mapping):
         :param group: an option OptGroup object or group name
         :raises: NoSuchOptError, NoSuchGroupError
         """
+        if default is None:
+            default = self._NoneValue()
         opt_info = self._get_opt_info(name, group)
         opt_info['default'] = default
 
+    @__clear_cache
+    def clear_override(self, name, group=None):
+        """Clear an override an opt value.
+
+        Clear a previously set override of the command line, config file
+        and default values of a given option.
+
+        :param name: the name/dest of the opt
+        :param group: an option OptGroup object or group name
+        :raises: NoSuchOptError, NoSuchGroupError
+        """
+        opt_info = self._get_opt_info(name, group)
+        opt_info['override'] = None
+
+    @__clear_cache
+    def clear_default(self, name, group=None):
+        """Clear an override an opt's default value.
+
+        Clear a previously set override of the default value of given option.
+
+        :param name: the name/dest of the opt
+        :param group: an option OptGroup object or group name
+        :raises: NoSuchOptError, NoSuchGroupError
+        """
+        opt_info = self._get_opt_info(name, group)
+        opt_info['default'] = None
+
     def _all_opt_infos(self):
         """A generator function for iteration opt infos."""
         for info in self._opts.values():
@@ -1347,8 +1381,11 @@ class ConfigOpts(collections.Mapping):
         info = self._get_opt_info(name, group)
         default, opt, override = [info[k] for k in sorted(info.keys())]
 
+        def _convert_none(value):
+            return None if isinstance(value, self._NoneValue) else value
+
         if override is not None:
-            return override
+            return _convert_none(override)
 
         values = []
         if self._cparser is not None:
@@ -1377,7 +1414,7 @@ class ConfigOpts(collections.Mapping):
             return values
 
         if default is not None:
-            return default
+            return _convert_none(default)
 
         return opt.default
 
diff --git a/tests/unit/test_cfg.py b/tests/unit/test_cfg.py
index 14a85c9..d5aa850 100644
--- a/tests/unit/test_cfg.py
+++ b/tests/unit/test_cfg.py
@@ -982,12 +982,32 @@ class ReparseTestCase(BaseTestCase):
 
 class OverridesTestCase(BaseTestCase):
 
+    def test_default_none(self):
+        self.conf.register_opt(StrOpt('foo', default='foo'))
+        self.conf([])
+        self.assertEquals(self.conf.foo, 'foo')
+        self.conf.set_default('foo', None)
+        self.assertEquals(self.conf.foo, None)
+        self.conf.clear_default('foo')
+        self.assertEquals(self.conf.foo, 'foo')
+
+    def test_override_none(self):
+        self.conf.register_opt(StrOpt('foo', default='foo'))
+        self.conf([])
+        self.assertEquals(self.conf.foo, 'foo')
+        self.conf.set_override('foo', None)
+        self.assertEquals(self.conf.foo, None)
+        self.conf.clear_override('foo')
+        self.assertEquals(self.conf.foo, 'foo')
+
     def test_no_default_override(self):
         self.conf.register_opt(StrOpt('foo'))
         self.conf([])
         self.assertEquals(self.conf.foo, None)
         self.conf.set_default('foo', 'bar')
         self.assertEquals(self.conf.foo, 'bar')
+        self.conf.clear_default('foo')
+        self.assertEquals(self.conf.foo, None)
 
     def test_default_override(self):
         self.conf.register_opt(StrOpt('foo', default='foo'))
@@ -995,7 +1015,7 @@ class OverridesTestCase(BaseTestCase):
         self.assertEquals(self.conf.foo, 'foo')
         self.conf.set_default('foo', 'bar')
         self.assertEquals(self.conf.foo, 'bar')
-        self.conf.set_default('foo', None)
+        self.conf.clear_default('foo')
         self.assertEquals(self.conf.foo, 'foo')
 
     def test_override(self):
@@ -1003,6 +1023,8 @@ class OverridesTestCase(BaseTestCase):
         self.conf.set_override('foo', 'bar')
         self.conf([])
         self.assertEquals(self.conf.foo, 'bar')
+        self.conf.clear_override('foo')
+        self.assertEquals(self.conf.foo, None)
 
     def test_group_no_default_override(self):
         self.conf.register_group(OptGroup('blaa'))
@@ -1011,6 +1033,8 @@ class OverridesTestCase(BaseTestCase):
         self.assertEquals(self.conf.blaa.foo, None)
         self.conf.set_default('foo', 'bar', group='blaa')
         self.assertEquals(self.conf.blaa.foo, 'bar')
+        self.conf.clear_default('foo', group='blaa')
+        self.assertEquals(self.conf.blaa.foo, None)
 
     def test_group_default_override(self):
         self.conf.register_group(OptGroup('blaa'))
@@ -1019,15 +1043,18 @@ class OverridesTestCase(BaseTestCase):
         self.assertEquals(self.conf.blaa.foo, 'foo')
         self.conf.set_default('foo', 'bar', group='blaa')
         self.assertEquals(self.conf.blaa.foo, 'bar')
-        self.conf.set_default('foo', None, group='blaa')
+        self.conf.clear_default('foo', group='blaa')
         self.assertEquals(self.conf.blaa.foo, 'foo')
 
     def test_group_override(self):
         self.conf.register_group(OptGroup('blaa'))
         self.conf.register_opt(StrOpt('foo'), group='blaa')
+        self.assertEquals(self.conf.blaa.foo, None)
         self.conf.set_override('foo', 'bar', group='blaa')
         self.conf([])
         self.assertEquals(self.conf.blaa.foo, 'bar')
+        self.conf.clear_override('foo', group='blaa')
+        self.assertEquals(self.conf.blaa.foo, None)
 
 
 class ResetAndClearTestCase(BaseTestCase):