Repository: jbasko/configmanager

Issue:
Move persistence code out of from Config completely, include in a class not part of v1

Discussion:

Patch:
diff --git a/configmanager/__init__.py b/configmanager/__init__.py
index 216dfa2..5051026 100644
--- a/configmanager/__init__.py
+++ b/configmanager/__init__.py
@@ -1,3 +1,3 @@
-__version__ = '1.1.7'
+__version__ = '1.1.8'
 
 from .v1 import *
diff --git a/configmanager/lightweight/managers.py b/configmanager/lightweight/managers.py
index 62fa678..b35729c 100644
--- a/configmanager/lightweight/managers.py
+++ b/configmanager/lightweight/managers.py
@@ -7,12 +7,12 @@ import six
 
 from configmanager.base import BaseSection, is_config_item
 from configmanager.lightweight.parsers import ConfigDeclarationParser
-from configmanager.persistence import ConfigParserMixin
+from configmanager.persistence import ConfigParserAdapter
 from configmanager.utils import not_set
 from .items import LwItem
 
 
-class LwConfig(ConfigParserMixin, BaseSection):
+class LwConfig(BaseSection):
     """
     Represents a collection of config items or sections of items
     which in turn are instances of Config.
@@ -24,17 +24,25 @@ class LwConfig(ConfigParserMixin, BaseSection):
     """
 
     cm__item_cls = LwItem
-    cm__config_parser_factory = configparser.ConfigParser
+    cm__configparser_factory = configparser.ConfigParser
 
-    def __new__(cls, config_declaration=None, item_cls=None):
+    def __new__(cls, config_declaration=None, item_cls=None, configparser_factory=None):
         instance = super(LwConfig, cls).__new__(cls)
+
         instance._cm__section = None
         instance._cm__configs = collections.OrderedDict()
+        instance._cm__configparser_adapter = None
+
         if item_cls:
             instance.cm__item_cls = item_cls
+        if configparser_factory:
+            instance.cm__configparser_factory = configparser_factory
+
         instance.cm__process_config_declaration = ConfigDeclarationParser(section=instance)
+
         if config_declaration:
             instance.cm__process_config_declaration(config_declaration)
+
         return instance
 
     def __repr__(self):
@@ -161,3 +169,16 @@ class LwConfig(ConfigParserMixin, BaseSection):
 
     def added_to_section(self, alias, section):
         self._cm__section = section
+
+    @property
+    def configparser(self):
+        """
+        Returns:
+            ConfigParserAdapter
+        """
+        if self._cm__configparser_adapter is None:
+            self._cm__configparser_adapter = ConfigParserAdapter(
+                config=self,
+                config_parser_factory=self.cm__configparser_factory,
+            )
+        return self._cm__configparser_adapter
diff --git a/configmanager/persistence.py b/configmanager/persistence.py
index 39d90f6..fbe41f4 100644
--- a/configmanager/persistence.py
+++ b/configmanager/persistence.py
@@ -1,14 +1,20 @@
+import configparser
+
 import six
 
 from configmanager.utils import not_set
 
 
-class ConfigParserMixin(object):
+class ConfigParserAdapter(object):
+    def __init__(self, config, config_parser_factory=None):
+        self.config = config
+        self.config_parser_factory = config_parser_factory or configparser.ConfigParser
+
     def read(self, *args, **kwargs):
         as_defaults = kwargs.pop('as_defaults', False)
 
         used_filenames = []
-        cp = self.cm__config_parser_factory()
+        cp = self.config_parser_factory()
 
         def get_filenames():
             for arg in args:
@@ -28,12 +34,12 @@ class ConfigParserMixin(object):
         return used_filenames
 
     def read_file(self, fileobj, as_defaults=False):
-        cp = self.cm__config_parser_factory()
+        cp = self.config_parser_factory()
         cp.read_file(fileobj)
         self.load_from_config_parser(cp, as_defaults=as_defaults)
 
     def read_string(self, string, source=not_set, as_defaults=False):
-        cp = self.cm__config_parser_factory()
+        cp = self.config_parser_factory()
         if source is not not_set:
             args = (string, source)
         else:
@@ -42,7 +48,7 @@ class ConfigParserMixin(object):
         self.load_from_config_parser(cp, as_defaults=as_defaults)
 
     def read_dict(self, dictionary, source=not_set, as_defaults=False):
-        cp = self.cm__config_parser_factory()
+        cp = self.config_parser_factory()
         if source is not not_set:
             args = (dictionary, source)
         else:
@@ -50,14 +56,14 @@ class ConfigParserMixin(object):
         cp.read_dict(*args)
         self.load_from_config_parser(cp, as_defaults=as_defaults)
 
-    def write(self, fileobj_or_path):
+    def write(self, fileobj_or_path, with_defaults=False):
         """
         Write configuration to a file object or a path.
 
         This differs from ``ConfigParser.write`` in that it accepts a path too.
         """
-        cp = self.cm__config_parser_factory()
-        self.load_into_config_parser(cp)
+        cp = self.config_parser_factory()
+        self.load_into_config_parser(cp, with_defaults=with_defaults)
 
         if isinstance(fileobj_or_path, six.string_types):
             with open(fileobj_or_path, 'w') as f:
@@ -70,27 +76,27 @@ class ConfigParserMixin(object):
             for option in cp.options(section):
                 value = cp.get(section, option)
                 if as_defaults:
-                    if section not in self:
-                        self[section] = self.__class__()
-                    if option not in self[section]:
-                        self[section][option] = self.cm__create_item(option, default=value)
+                    if section not in self.config:
+                        self.config.cm__add_section(section, self.config.cm__create_section())
+                    if option not in self.config[section]:
+                        self.config[section].cm__add_item(option, self.config.cm__create_item(option, default=value))
                     else:
-                        self[section][option].default = value
+                        self.config[section][option].default = value
                 else:
-                    if section not in self:
+                    if section not in self.config:
                         continue
-                    if option not in self[section]:
+                    if option not in self.config[section]:
                         continue
-                    self[section][option].value = value
+                    self.config[section][option].value = value
 
-    def load_into_config_parser(self, cp):
-        for item_path, item in self.iter_items():
+    def load_into_config_parser(self, cp, with_defaults=False):
+        for item_path, item in self.config.iter_items():
             if len(item_path) > 2:
                 raise RuntimeError(
                     '{cls} with more than 2 path segments cannot be loaded into ConfigParser'.format(
                         cls=item.__class__.__name__,
                 ))
-            if item.is_default:
+            if not with_defaults and item.is_default:
                 continue
 
             if len(item_path) == 2:


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 634b37c..e69de29 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,39 +0,0 @@
-import pytest
-
-from configmanager.v1 import Config
-
-
-@pytest.fixture
-def simple_config():
-    return Config({
-        'simple': {
-            'str': '',
-            'int': 0,
-            'float': 0.0,
-        },
-        'random': {
-            'name': 'Bob',
-        },
-    })
-
-
-@pytest.fixture
-def empty_config_file(tmpdir):
-    path = tmpdir.join('empty.ini')
-    with open(path.strpath, 'w') as f:
-        f.write('')
-    return path.strpath
-
-
-@pytest.fixture
-def simple_config_file(tmpdir):
-    path = tmpdir.join('simple.ini')
-    with open(path.strpath, 'w') as f:
-        f.write('[simple]\n')
-        f.write('str = hello\n')
-        f.write('int = 5\n')
-        f.write('float = 33.33\n')
-        f.write('\n')
-        f.write('[random]\n')
-        f.write('name = Johnny')
-    return path.strpath
diff --git a/tests/test_persistence.py b/tests/test_persistence.py
index da9e2a2..a42268f 100644
--- a/tests/test_persistence.py
+++ b/tests/test_persistence.py
@@ -3,9 +3,45 @@ import pytest
 from configmanager.v1 import Config, Item
 
 
+@pytest.fixture
+def simple_config():
+    return Config({
+        'simple': {
+            'str': '',
+            'int': 0,
+            'float': 0.0,
+        },
+        'random': {
+            'name': 'Bob',
+        },
+    })
+
+
+@pytest.fixture
+def empty_config_file(tmpdir):
+    path = tmpdir.join('empty.ini')
+    with open(path.strpath, 'w') as f:
+        f.write('')
+    return path.strpath
+
+
+@pytest.fixture
+def simple_config_file(tmpdir):
+    path = tmpdir.join('simple.ini')
+    with open(path.strpath, 'w') as f:
+        f.write('[simple]\n')
+        f.write('str = hello\n')
+        f.write('int = 5\n')
+        f.write('float = 33.33\n')
+        f.write('\n')
+        f.write('[random]\n')
+        f.write('name = Johnny')
+    return path.strpath
+
+
 def test_reads_empty_config_from_file_obj(simple_config, empty_config_file):
     with open(empty_config_file) as f:
-        simple_config.read_file(f)
+        simple_config.configparser.read_file(f)
 
     assert simple_config.to_dict() == {
         'simple': {
@@ -21,7 +57,7 @@ def test_reads_empty_config_from_file_obj(simple_config, empty_config_file):
 
 def test_reads_simple_config_from_file_obj(simple_config, simple_config_file):
     with open(simple_config_file) as f:
-        simple_config.read_file(f)
+        simple_config.configparser.read_file(f)
 
     assert simple_config.to_dict() == {
         'simple': {
@@ -43,7 +79,7 @@ def test_writes_config_to_file(tmpdir):
     })
     config_path = tmpdir.join('config1.ini').strpath
     with open(config_path, 'w') as f:
-        m.write(f)
+        m.configparser.write(f)
 
     # default value shouldn't be written
     with open(config_path) as f:
@@ -52,7 +88,7 @@ def test_writes_config_to_file(tmpdir):
     m['random']['name'].value = 'Harry'
 
     with open(config_path, 'w') as f:
-        m.write(f)
+        m.configparser.write(f)
 
     with open(config_path) as f:
         assert f.read() == '[random]\nname = Harry\n\n'
@@ -72,12 +108,12 @@ def test_preserves_bool_notation(tmpdir):
         f.write('[flags]\nenabled = Yes\n\n')
 
     with open(config_path) as f:
-        m.read_file(f)
+        m.configparser.read_file(f)
 
     assert m.flags.enabled.value is True
 
     with open(config_path, 'w') as f:
-        m.write(f)
+        m.configparser.write(f)
 
     with open(config_path) as f:
         assert f.read() == '[flags]\nenabled = Yes\n\n'
@@ -95,7 +131,7 @@ def test_configparser_writer_does_not_accept_three_deep_paths(tmpdir):
 
     with pytest.raises(RuntimeError):
         with open(config_path, 'w') as f:
-            m.write(f)
+            m.configparser.write(f)
 
 
 def test_read_reads_multiple_files_in_order(tmpdir):
@@ -115,29 +151,29 @@ def test_read_reads_multiple_files_in_order(tmpdir):
     path3 = tmpdir.join('config3.ini').strpath
 
     # Empty file
-    m.write(path1)
+    m.configparser.write(path1)
 
     # Can read from one empty file
-    m.read(path1)
+    m.configparser.read(path1)
     assert m.is_default
 
     m.a.x.value = 0.33
     m.b.n.value = 42
-    m.write(path2)
+    m.configparser.write(path2)
 
     # Can read from one non-empty file
     m.reset()
-    m.read(path2)
+    m.configparser.read(path2)
     assert not m.is_default
     assert m.a.x.value == 0.33
 
     m.reset()
     m.a.x.value = 0.66
     m.b.m.value = 'YES'
-    m.write(path3)
+    m.configparser.write(path3)
 
     m.reset()
-    m.read([path1, path2, path3])
+    m.configparser.read([path1, path2, path3])
 
     assert m.a.x.value == 0.66
     assert m.a.y.is_default
@@ -146,7 +182,7 @@ def test_read_reads_multiple_files_in_order(tmpdir):
     assert m.b.n.value == 42
 
     m.reset()
-    m.read([path3, path2, path1])
+    m.configparser.read([path3, path2, path1])
 
     assert m.a.x.value == 0.33  # this is the only difference with the above order
     assert m.a.y.is_default
@@ -156,7 +192,7 @@ def test_read_reads_multiple_files_in_order(tmpdir):
 
     # Make sure multiple paths supported in non-list syntax.
     m.reset()
-    m.read(path3, path2, path1)
+    m.configparser.read(path3, path2, path1)
 
     assert m.a.x.value == 0.33
     assert m.b.m.value is True
@@ -172,7 +208,7 @@ def test_read_string():
         },
     })
 
-    m.read_string(u'[a]\nx = haha\ny = yaya\n')
+    m.configparser.read_string(u'[a]\nx = haha\ny = yaya\n')
     assert m.a.x.value == 'haha'
     assert m.a.y.value == 'yaya'
 
@@ -187,7 +223,7 @@ def test_read_dict():
         },
     })
 
-    m.read_dict({
+    m.configparser.read_dict({
         'a': {'x': 'xoxo', 'y': 'yaya'},
         'b': {'m': 'mama', 'n': 'nono'},
     })
@@ -205,7 +241,7 @@ def test_read_as_defaults_treats_all_values_as_declarations(tmpdir):
         f.write('[messages]\ngreeting = Hello, home!\n')
 
     m = Config()
-    m.read(path, as_defaults=True)
+    m.configparser.read(path, as_defaults=True)
 
     assert m.uploads
     assert m.uploads.threads.value == '5'
@@ -216,11 +252,32 @@ def test_read_as_defaults_treats_all_values_as_declarations(tmpdir):
 
     # Reading again with as_defaults=True should not change the values, only the defaults
     m.uploads.threads.value = '55'
-    m.read(path, as_defaults=True)
+    m.configparser.read(path, as_defaults=True)
     assert m.uploads.threads.value == '55'
     assert m.uploads.threads.default == '5'
 
     # But reading with as_defaults=False should change the value
-    m.read(path)
+    m.configparser.read(path)
     assert m.uploads.threads.value == '5'
     assert m.uploads.threads.default == '5'
+
+
+def test_write_with_defaults_writes_defaults_too(tmpdir):
+    path = tmpdir.join('conf.ini').strpath
+
+    m = Config({'a': {'b': 1, 'c': 'd', 'e': True}})
+
+    m.configparser.write(path)
+    with open(path, 'r') as f:
+        assert len(f.read()) == 0
+
+    m.configparser.write(path, with_defaults=True)
+    with open(path, 'r') as f:
+        assert len(f.read()) > 0
+
+    n = Config()
+    n.configparser.read(path, as_defaults=True)
+
+    assert n.a.b.value == '1'  # it is a string because n has no idea about types
+    assert n.a.e.value == 'True'  # same
+    assert n.a.c.value == 'd'
diff --git a/tests/test_v1.py b/tests/test_v1.py
index cfdfd09..df2cd3c 100644
--- a/tests/test_v1.py
+++ b/tests/test_v1.py
@@ -165,3 +165,28 @@ def test_exceptions():
     password = Item('password', required=True)
     with pytest.raises(ConfigValueMissing):
         assert not password.value
+
+
+def test_configparser_integration(tmpdir):
+    defaults_ini_path = tmpdir.join('defaults.ini').strpath
+    custom_ini_path = tmpdir.join('custom.ini').strpath
+
+    # Config sections expose ConfigParser adapter as configparser property:
+    config = Config()
+
+    # assuming that defaults.ini exists, this would initialise Config
+    # with all values mentioned in defaults.ini set as defaults.
+    # Just like with ConfigParser, this won't fail if the file does not exist.
+    config.configparser.read(defaults_ini_path, as_defaults=True)
+
+    # if you have already declared defaults, you can load custom
+    # configuration without specifying as_defaults=True:
+    config.configparser.read(custom_ini_path)
+
+    # other ConfigParser-like methods such as read_dict, read_string, read_file are provided too.
+    # when you are done setting config values, you can write them to file too.
+    config.configparser.write(custom_ini_path)
+
+    # Note that default values won't be written unless you explicitly request it
+    # by passing with_defaults=True
+    config.configparser.write(custom_ini_path, with_defaults=True)