Restructure container dict and list specs

Update how defaults and optional specs are defined, and allow for more
concise spec definitions

configspec/manager.py

@@ -16,12 +16,16 @@ class ConfigManager():
         """
         Save the current state of the ConfigManager for future restoration with ``fallback()``.
         Includes the loaded config source, the validated config, any added config specifications,
-        and any frozen values.
+        and any frozen values. Returns a state dict that may be passed back to `fallback()` to
+        restore a particular state.
         """
-        self._saved_state["config_source"] = deepcopy(self.config_source)
-        self._saved_state["root_config"] = deepcopy(self.root_config)
-        self._saved_state["confspecs"] = deepcopy(self.confspecs)
-        self._saved_state["frozen_config"] = deepcopy(self.frozen_config)
+        new_saved_state = {}
+        new_saved_state["config_source"] = deepcopy(self.config_source)
+        new_saved_state["root_config"] = deepcopy(self.root_config)
+        new_saved_state["confspecs"] = deepcopy(self.confspecs)
+        new_saved_state["frozen_config"] = deepcopy(self.frozen_config)
+        self._saved_state = new_saved_state
+        return new_saved_state
 
     def fallback(self):
         """

configspec/specification.py

@@ -4,6 +4,8 @@ being loaded and used.
 """
 
 from abc import ABC, abstractmethod
+import re
+import itertools
 
 from preserve import preservable
 
@@ -16,9 +18,12 @@ class InvalidConfigError(Exception):
 
 
 class _ValueSpecification(ABC):
-    def __init__(self, default=None, optional=False, helptext=""):
-        self.default = default
-        self.optional = optional
+    """
+    The abstract base class for all ConfigSpecifications. `default` values are used to ensure
+    that a config structure exists even if optional fields are not supplied.
+    """
+
+    def __init__(self, helptext=""):
         self.helptext = helptext
 
     @abstractmethod
@@ -32,8 +37,8 @@ class _ValueSpecification(ABC):
 
 @preservable
 class BoolSpec(_ValueSpecification):
-    def __init__(self, default=None, optional=False, helptext=""):
-        super().__init__(default, optional, helptext)
+    def __init__(self, helptext=""):
+        super().__init__(helptext)
 
     def validate(self, value):
         if not isinstance(value, bool):
@@ -42,9 +47,8 @@ class BoolSpec(_ValueSpecification):
 
 @preservable
 class IntSpec(_ValueSpecification):
-    def __init__(self, default=None, minval=None, maxval=None,
-                 optional=False, helptext=""):
-        super().__init__(default, optional, helptext)
+    def __init__(self, minval=None, maxval=None, helptext=""):
+        super().__init__(helptext)
         self.minval = minval
         self.maxval = maxval
 
@@ -61,74 +65,121 @@ class IntSpec(_ValueSpecification):
 
 @preservable
 class StringSpec(_ValueSpecification):
-    def __init__(self, default="", minlength=None, maxlength=None,
-                 optional=False, helptext=""):
-        super().__init__(default, optional, helptext)
-        self.minlength = minlength
-        self.maxlength = maxlength
+    def __init__(self, regex=None, helptext=""):
+        """
+        Specify a string to be provided. If `regex` is not None, validation requires that the
+        config value directly matches the supplied regex string (calls `re.fullmatch`)
+        """
+        super().__init__(helptext)
+        self.regex = regex
 
     def validate(self, value):
         if not isinstance(value, str):
             raise InvalidConfigError(F"Config value must be a string and is {value}")
-        if self.minlength is not None and len(value) < self.minlength:
-            raise InvalidConfigError("Config string length must be >= " +
-                                     str(self.minlength))
-        if self.maxlength is not None and len(value) > self.maxlength:
-            raise InvalidConfigError("Config string length must be <= " +
-                                     str(self.maxlength))
+        if self.regex and (re.fullmatch(self.regex, value) is None):
+            raise InvalidConfigError(F"Config string '{value}' must match regex"
+                                     F" pattern '{self.regex}'")
 
 
 @preservable
 class DictSpec(_ValueSpecification):
-    def __init__(self, default=None, optional=False, helptext=""):
-        super().__init__(default, optional, helptext)
+    def __init__(self, helptext=""):
+        super().__init__(helptext)
         self.spec_dict = {}
+        self.optional_spec_dict = {}
+        self.default_values = {}
+
+    def add_spec(self, name, value_spec, optional=False, default=None):
+        """
+        Adds a specification to this dict with the given `name`. To validate, corresponding fields
+        must be present with matching keys. Only supports string names/keys.
+
+        Specifications marked as `optional` will be filled with their default value if not present
+        during validation (note, this _will_ modify the data validated to fill in defaults), and
+        will also pass validation if the value is None. Providing `default` as anything other than
+        None will implicitly set `optional` to True.
 
-    def add_spec(self, name, newspec):
-        if not isinstance(newspec, _ValueSpecification):
+        The default value must be None, or a value that will pass validation with the spec.
+
+        To allow concise access to the value_spec passed in, returns `value_spec`. This beahviour 
+        has an exception for ListSpecs, for which it will return the value_spec passed to the 
+        ListSpec itself (also to allow concise access to further define the spec).
+
+        Note that nested optional dicts _are_ possible by passing `default` as an empty dict `{}`,
+        and passing `value_spec` a DictSpec where every field is itself optional. If this entire
+        field is missing during validation it will then attempt to validate an empty dict,
+        populating it with all of the nested optional fields.
+        """
+        if not isinstance(value_spec, _ValueSpecification):
             raise TypeError("Config specification must be an instance of a "
                             "_ValueSpecification subclass")
         if not isinstance(name, str):
             raise TypeError("Config specification name must be a string")
-        self.spec_dict[name] = newspec
-        return newspec
+        if (name in self.spec_dict) or (name in self.optional_spec_dict):
+            raise TypeError(F"Name '{name}' already exists in this DictSpec")
+
+        if default is not None:
+            optional = True
+
+        if not optional:
+            self.spec_dict[name] = value_spec
+        else:
+            self.optional_spec_dict[name] = value_spec
+            self.default_values[name] = default
 
-    def add_specs(self, spec_list):
+        if isinstance(value_spec, ListSpec):
+            return value_spec.spec
+
+        return value_spec
+
+    def add_specs(self, spec_dict, optional=False):
         """
-        Convenience method to pass a series of specifications to 'add_spec()'
+        Convenience method to pass a series of specifications to 'add_spec()'.
 
         Args:
-            spec_list: A list of tuples, where each tuple contains a `name` and `newspec` value,
-                to be passed in the same form as a call to `add_spec`
+            spec_dict: A dict of specs, where each key is the name of the spec stored as the
+                corresponding value. If `optional` is true, the value may be a tuple with the
+                first item being the spec and the second being the default value.
+            optional: Same behaviour as `add_spec()`. Applies to all specs passed in, and will use
+                a default value of None.
         """
-        for spec in spec_list:
-            self.add_spec(*spec)
+        for name, spec in spec_dict.items():
+            if optional and isinstance(spec, tuple):
+                self.add_spec(name, spec[0], optional=True, default=spec[1])
+            else:
+                self.add_spec(name, spec, optional)
 
     def validate(self, value_dict):
         """
-        Checks the supplied value to confirm that it complies with this specification.
+        Checks the supplied values to confirm that it complies with this specification.
         Raises InvalidConfigError on failure.
 
-        This *can* modify the supplied value dict, inserting defaults for any child
-        config specifications that are marked as optional.
+        This _can_ modify the supplied value dict, inserting defaults for any child
+        config specifications that are marked as optional. Optional values (after defaults are
+        inserted) must either be None or pass validation as usual.
         """
-        spec_set = set(self.spec_dict.keys())
-        value_set = set(value_dict.keys())
+        for missing_key in (self.spec_dict.keys() - value_dict.keys()):
+            raise InvalidConfigError(F"Dict must contain key '{missing_key}'")
 
-        for missing_key in spec_set - value_set:
-            if not self.spec_dict[missing_key].optional:
-                raise InvalidConfigError("Dict must contain key: " +
-                                         missing_key)
-            else:
-                value_dict[missing_key] = self.spec_dict[missing_key].default
+        for extra_key in ((value_dict.keys() - self.spec_dict.keys()) -
+                          self.optional_spec_dict.keys()):
+            raise InvalidConfigError(F"Dict contains unknown key '{extra_key}'")
+
+        for default_key in (self.optional_spec_dict.keys() - value_dict.keys()):
+            value_dict[default_key] = self.default_values[default_key]
 
-        for extra_key in value_set - spec_set:
-            raise InvalidConfigError("Dict contains unknown key: " +
-                                     extra_key)
+        for key, spec in self.spec_dict.items():
+            try:
+                spec.validate(value_dict[key])
+            except InvalidConfigError as e:
+                e.args = ("Key: " + key,) + e.args
+                raise
 
-        for key, value in value_dict.items():
+        for key, spec in self.optional_spec_dict.items():
+            if value_dict[key] is None:
+                continue
             try:
-                self.spec_dict[key].validate(value)
+                spec.validate(value_dict[key])
             except InvalidConfigError as e:
                 e.args = ("Key: " + key,) + e.args
                 raise
@@ -147,53 +198,58 @@ class DictSpec(_ValueSpecification):
                 with this config specification.
         """
         template = {}
-        for key, confspec in self.spec_dict.items():
-            if confspec.optional and (not include_optional):
-                continue
+        if include_optional:
+            spec_items = itertools.chain(self.spec_dict.items(), self.optional_spec_dict.items())
+        else:
+            spec_items = self.spec_dict.items()
 
+        for key, confspec in spec_items:
             if hasattr(confspec, "get_template"):
                 template[key] = confspec.get_template(include_optional)
             else:
-                template[key] = confspec.default
+                template[key] = self.default_values.get(key, None)
         return template
 
 
-class _ListSpecMixin():
+@preservable
+class ListSpec(_ValueSpecification):
+    """
+    Specify a list of values
+    """
+
+    def __init__(self, value_spec, min_values=None, max_values=None, helptext=""):
+        """
+        Make a new ListSpec. Requires a child spec to be applied to all list values.
+        Min and max limits can be supplied to ensure a certain range of list sizes.
+        Returns the value_spec passed in, to allow easy additions to it (if it's a DictSpec)
+        """
+        super().__init__(helptext)
+        self.spec = value_spec
+        self.min_values = min_values
+        self.max_values = max_values
+
     def validate(self, value_list):
         if not isinstance(value_list, list):
             raise InvalidConfigError("Config item must be a list")
+        if self.min_values and (len(value_list) < self.min_values):
+            raise InvalidConfigError(F"Config list requires {self.min_values} values, and only"
+                                     F" has {len(value_list)}")
+        if self.max_values and (len(value_list) > self.max_values):
+            raise InvalidConfigError(F"Config list can have no more than {self.max_values} values,"
+                                     F" and has {len(value_list)}")
+
         for index, value in enumerate(value_list):
             try:
-                super().validate(value)
+                self.spec.validate(value)
             except InvalidConfigError as e:
                 e.args = ("List index: " + str(index),) + e.args
                 raise
 
     def get_template(self, include_optional=False):
-        if hasattr(super(), "get_template"):
-            return [super().get_template(include_optional)]
+        if hasattr(self.spec, "get_template"):
+            return [self.spec.get_template(include_optional)]
         else:
-            return [self.default]
-
-
-@preservable
-class BoolListSpec(_ListSpecMixin, BoolSpec):
-    pass
-
-
-@preservable
-class IntListSpec(_ListSpecMixin, IntSpec):
-    pass
-
-
-@preservable
-class StringListSpec(_ListSpecMixin, StringSpec):
-    pass
-
-
-@preservable
-class DictListSpec(_ListSpecMixin, DictSpec):
-    pass
+            return [None]
 
 
 @preservable

tests/test_configspec.py

@@ -2,42 +2,29 @@
 from copy import deepcopy
 import pytest
 import preserve
-import configspec
+from configspec import *
 
 
 @pytest.fixture
 def example_spec():
-    confspec = configspec.ConfigSpecification()
-    confspec.add_spec("bool_a",
-                      configspec.BoolSpec(False, False, "Bool A"))
-    confspec.add_spec("boollist_a",
-                      configspec.BoolListSpec(False, False, "Bool A"))
-    confspec.add_spec("int_a",
-                      configspec.IntSpec(0, -4, 4, False, "Int A"))
-    confspec.add_spec("intlist_a",
-                      configspec.IntListSpec(0, -4, 4, False, "Int A"))
-    confspec.add_spec("string_a",
-                      configspec.StringSpec("ThisIsAString", 4, 15, False, "String A"))
-    confspec.add_spec("stringlist_a",
-                      configspec.StringListSpec("ThisIsAString", 4, 15, False, "String A"))
-
-    confdictspec = confspec.add_spec("dict_a",
-                                     configspec.DictSpec(None, False, "Dict A"))
-    confdictspec.add_spec("bool_b",
-                          configspec.BoolSpec(False, False, "Bool B"))
-    confdictspec.add_spec("int_b",
-                          configspec.IntSpec(0, -4, 4, False, "Int B"))
-    confdictspec.add_spec("string_b",
-                          configspec.StringSpec("ThisIsAString", 4, 15, False, "String B"))
-
-    confdictlist = confspec.add_spec("dictlist_a",
-                                     configspec.DictListSpec(None, False, "List of Dicts B"))
-    confdictlist.add_spec("bool_c",
-                          configspec.BoolSpec(False, False, "Bool C"))
-    confdictlist.add_spec("int_c",
-                          configspec.IntSpec(0, -4, 4, False, "Int C"))
-    confdictlist.add_spec("string_c",
-                          configspec.StringSpec("ThisIsAString", 4, 15, False, "String C"))
+    confspec = ConfigSpecification()
+    confspec.add_spec("bool_a", BoolSpec(helptext="Bool A"))
+    confspec.add_spec("boollist_a", ListSpec(BoolSpec(), helptext="Bool List A"))
+    confspec.add_spec("int_a", IntSpec(-4, 4, helptext="Int A"))
+    confspec.add_spec("intlist_a", ListSpec(IntSpec(-4, 4, "Int A")))
+    confspec.add_spec("string_a", StringSpec(helptext="String A"))
+    confspec.add_spec("stringlist_a", ListSpec(StringSpec(), helptext="String List A"))
+
+    confdictspec = confspec.add_spec("dict_a", DictSpec(helptext="Dict A"))
+    confdictspec.add_spec("bool_b", BoolSpec(helptext="Bool B"))
+    confdictspec.add_spec("int_b", IntSpec(helptext="Int B"))
+    confdictspec.add_spec("string_b", StringSpec(helptext="String B"))
+
+    confdictlist = confspec.add_spec("dictlist_a", ListSpec(
+        DictSpec(), helptext="List of Dicts B"))
+    confdictlist.add_specs({"bool_c": BoolSpec(helptext="Bool C"),
+                            "int_c": IntSpec(helptext="Int C"),
+                            "string_c": StringSpec(helptext="String C")})
 
     return confspec
 
@@ -55,9 +42,9 @@ def example_values():
                 'int_b': 0,
                 'string_b': 'texttesttext'},
             'dictlist_a': [{'bool_c': False,
-                            'int_c': 0,
+                            'int_c': 3,
                             'string_c': 'string1'},
-                           {'bool_c': False,
+                           {'bool_c': True,
                             'int_c': 0,
                             'string_c': 'string2'}]}
 
@@ -65,76 +52,69 @@ def example_values():
 
 
 def test_addspec(example_spec):
-    confspec = configspec.ConfigSpecification()
-    confspec.add_specs([
-        ("bool_a", configspec.BoolSpec(False, False, "Bool A")),
-        ("boollist_a", configspec.BoolListSpec(False, False, "Bool A")),
-        ("int_a", configspec.IntSpec(0, -4, 4, False, "Int A")),
-        ("intlist_a", configspec.IntListSpec(0, -4, 4, False, "Int A")),
-        ("string_a", configspec.StringSpec("ThisIsAString", 4, 15, False, "String A")),
-        ("stringlist_a", configspec.StringListSpec("ThisIsAString", 4, 15, False, "String A"))
-    ])
-
-    confdictspec = confspec.add_spec("dict_a", configspec.DictSpec(None, False, "Dict A"))
-
-    confdictspec.add_specs([
-        ("bool_b", configspec.BoolSpec(False, False, "Bool B")),
-        ("int_b", configspec.IntSpec(0, -4, 4, False, "Int B")),
-        ("string_b", configspec.StringSpec("ThisIsAString", 4, 15, False, "String B"))
-    ])
+    # Most of the test is really just forming the fixture without exception
+    assert isinstance(example_spec, ConfigSpecification)
 
-    confdictlist = confspec.add_spec("dictlist_a",
-                                     configspec.DictListSpec(None, False, "List of Dicts B"))
 
-    confdictlist.add_specs([
-        ("bool_c", configspec.BoolSpec(False, False, "Bool C")),
-        ("int_c", configspec.IntSpec(0, -4, 4, False, "Int C")),
-        ("string_c", configspec.StringSpec("ThisIsAString", 4, 15, False, "String C"))
-    ])
-
-    # Preserve to be able to directly compare resulting confspec structure
-    assert preserve.preserve(example_spec) == preserve.preserve(confspec)
+def test_preserveable(example_spec):
+    assert isinstance(preserve.preserve(example_spec), dict)
 
 
 def test_config_validate(example_spec, example_values):
-    spec = example_spec
-    spec.validate(example_values)
+    example_spec.validate(example_values)
 
 
 def test_invalid_bool():
-    spec = configspec.BoolSpec()
-    with pytest.raises(configspec.InvalidConfigError, match="value must be a boolean"):
+    spec = BoolSpec()
+    with pytest.raises(InvalidConfigError, match="value must be a boolean"):
         spec.validate("false")
-    with pytest.raises(configspec.InvalidConfigError, match="value must be a boolean"):
+    with pytest.raises(InvalidConfigError, match="value must be a boolean"):
         spec.validate(None)
 
 
 def test_invalid_int():
-    spec = configspec.IntSpec(minval=-4, maxval=4)
-    with pytest.raises(configspec.InvalidConfigError, match="value must be >="):
+    spec = IntSpec(minval=-4, maxval=4)
+    with pytest.raises(InvalidConfigError, match="value must be >="):
         spec.validate(-5)
-    with pytest.raises(configspec.InvalidConfigError, match="value must be <="):
+    with pytest.raises(InvalidConfigError, match="value must be <="):
         spec.validate(5)
-    with pytest.raises(configspec.InvalidConfigError, match="value must be an integer"):
+    with pytest.raises(InvalidConfigError, match="value must be an integer"):
         spec.validate("5")
-    with pytest.raises(configspec.InvalidConfigError, match="value must be an integer"):
+    with pytest.raises(InvalidConfigError, match="value must be an integer"):
         spec.validate(None)
 
 
 def test_invalid_string():
-    spec = configspec.StringSpec(minlength=4, maxlength=15)
-    with pytest.raises(configspec.InvalidConfigError, match="string length must be >="):
-        spec.validate("123")
-    with pytest.raises(configspec.InvalidConfigError, match="string length must be <="):
-        spec.validate("1234567890abcdef")
-    with pytest.raises(configspec.InvalidConfigError, match="value must be a string"):
+    spec = StringSpec()
+    # TODO add regex check
+    with pytest.raises(InvalidConfigError, match="value must be a string"):
         spec.validate(1)
-    with pytest.raises(configspec.InvalidConfigError, match="value must be a string"):
+    with pytest.raises(InvalidConfigError, match="value must be a string"):
         spec.validate(None)
 
 
+def test_dict_optional():
+    spec = DictSpec()
+    spec.add_spec("int1", IntSpec())
+    spec.add_spec("int2", IntSpec(), optional=True)
+    spec.add_spec("int3", IntSpec(), optional=True, default=1)
+    with pytest.raises(InvalidConfigError, match="must contain key 'int1'"):
+        spec.validate({})
+
+    with pytest.raises(InvalidConfigError, match="contains unknown key"):
+        spec.validate({"int1": 1, "int5": 0})
+
+    d = {"int1": 1, "int2": 2, "int3": 3}
+    spec.validate(d)
+    assert d == {"int1": 1, "int2": 2, "int3": 3}
+
+    d = {"int1": 1}
+    spec.validate(d)
+    assert d == {"int1": 1, "int2": None, "int3": 1}
+
+
 def test_man(example_spec, example_values):
-    confman = configspec.ConfigManager()
+    confman = ConfigManager()
     confman.add_confspec("test_bundle", example_spec)
     confman.load({"test_bundle": example_values})
     assert confman.get_config_bundle("test_bundle") == example_values