From fc0f27285fe7153a0696102d0852429d0d843ff2 Mon Sep 17 00:00:00 2001
From: Doug Hellmann <doug@doughellmann.com>
Date: Wed, 3 Oct 2018 14:52:59 -0400
Subject: [PATCH] fix formatting of sample defaults

Use six.text_type instead of str.

Recursively format the elements of a list based on the item type for
the list, if available.

Apply the formatting method to the sample default value in case that
value is not a string.

Change-Id: Ida873ad6a875b608e9e69fd4caf118fde0837dbb
Signed-off-by: Doug Hellmann <doug@doughellmann.com>
Co-Authored-By: Ben Nemec <bnemec@redhat.com>
---
 oslo_config/tests/test_types.py | 37 +++++++++++++++++++++++++++++++++
 oslo_config/types.py            | 15 ++++++++-----
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/oslo_config/tests/test_types.py b/oslo_config/tests/test_types.py
index 13ac6507..78e23b50 100644
--- a/oslo_config/tests/test_types.py
+++ b/oslo_config/tests/test_types.py
@@ -1039,3 +1039,40 @@ class PortTypeTests(TypeTestHelper, unittest.TestCase):
         t = types.Port(max=0)
         self.assertRaises(ValueError, t, 1)
         t(0)
+
+
+class FormatSampleDefaultTests(unittest.TestCase):
+    def test_string(self):
+        t = types.String()
+        self.assertEqual([' bar '],
+                         t.format_defaults('foo', sample_default=' bar '))
+
+    def test_string_non_str(self):
+        t = types.String()
+        e = Exception('bar')
+        self.assertEqual(['bar'],
+                         t.format_defaults('', sample_default=e))
+
+    def test_string_non_str_spaces(self):
+        t = types.String()
+        e = Exception(' bar ')
+        self.assertEqual(['" bar "'],
+                         t.format_defaults('', sample_default=e))
+
+    def test_list_string(self):
+        t = types.List(item_type=types.String())
+        test_list = ['foo', Exception(' bar ')]
+        self.assertEqual(['foo," bar "'],
+                         t.format_defaults('', sample_default=test_list))
+
+    def test_list_no_type(self):
+        t = types.List()
+        test_list = ['foo', Exception(' bar ')]
+        self.assertEqual(['foo," bar "'],
+                         t.format_defaults('', sample_default=test_list))
+
+    def test_list_not_list(self):
+        t = types.List()
+        self.assertEqual(['foo'],
+                         t.format_defaults('',
+                                           sample_default=Exception('foo')))
diff --git a/oslo_config/types.py b/oslo_config/types.py
index 4cfe527c..a973d0e0 100644
--- a/oslo_config/types.py
+++ b/oslo_config/types.py
@@ -44,7 +44,10 @@ class ConfigType(object):
 
         """
         if sample_default is not None:
-            default_str = sample_default
+            if isinstance(sample_default, six.string_types):
+                default_str = sample_default
+            else:
+                default_str = self._formatter(sample_default)
         elif default is None:
             default_str = self.NONE_DEFAULT
         else:
@@ -348,7 +351,7 @@ class Number(ConfigType):
         )
 
     def _formatter(self, value):
-        return str(value)
+        return six.text_type(value)
 
 
 class Integer(Number):
@@ -458,7 +461,8 @@ class List(ConfigType):
     comma and next item until validation succeeds or there is no parts left.
     In the later case it will signal validation error.
 
-    :param item_type: type of list items
+    :param item_type: Type of list items. Should be an instance of
+                      ``ConfigType``.
     :param bounds: if True, value should be inside "[" and "]" pair
     :param type_name: Type name to be used in the sample config file.
 
@@ -531,10 +535,11 @@ class List(ConfigType):
             return fmtstr.format(value)
         if isinstance(value, list):
             value = [
-                six.text_type(v)
+                self.item_type._formatter(v)
                 for v in value
             ]
-        return fmtstr.format(','.join(value))
+            return fmtstr.format(','.join(value))
+        return fmtstr.format(self.item_type._formatter(value))
 
 
 class Range(ConfigType):