From 7dd091d40910100d0cfabb6657375f3f0fecccab Mon Sep 17 00:00:00 2001 From: Jazeps Basko Date: Sat, 10 Jun 2017 07:51:07 +0100 Subject: [PATCH] Configuration declaration parser rewritten: * empty list is interpreted as a default value of list type item * empty dictionary is interpreted as a default value of a dict type item * list of anything but tuples is treated as a default value of list type item * no more class-based configurations * no more free-floating Item() instances in lists used for section declaration -- must be a tuple. --- configmanager/base.py | 2 + configmanager/config_declaration_parser.py | 96 +++++++ configmanager/managers.py | 10 +- configmanager/parsers.py | 108 ------- configmanager/persistence.py | 3 + configmanager/sections.py | 4 + tests/test_config.py | 40 --- tests/test_config_declaration_parser.py | 312 ++++++++++++--------- tests/test_hooks.py | 4 +- tests/test_iterators.py | 4 +- tests/test_v1.py | 13 - 11 files changed, 288 insertions(+), 308 deletions(-) create mode 100644 configmanager/config_declaration_parser.py delete mode 100644 configmanager/parsers.py diff --git a/configmanager/base.py b/configmanager/base.py index 9aba5b0..f74fc75 100644 --- a/configmanager/base.py +++ b/configmanager/base.py @@ -4,6 +4,7 @@ class BaseItem(object): is_item = True is_section = False + is_config = False def is_config_item(obj): @@ -18,6 +19,7 @@ class BaseSection(object): is_item = False is_section = True + is_config = False def add_item(self, alias, item): raise NotImplementedError() diff --git a/configmanager/config_declaration_parser.py b/configmanager/config_declaration_parser.py new file mode 100644 index 0000000..f6ac817 --- /dev/null +++ b/configmanager/config_declaration_parser.py @@ -0,0 +1,96 @@ +import collections +import inspect + +import six + +from configmanager.base import BaseItem, BaseSection + + +def parse_config_declaration(declaration, parent_section=None, root=None): + + # Pass _configmanager_settings through so we know how to create new items/sections + # Process declaration recursively (unlike the old parser) + # Don't allow much freedom into what is accepted + # Sequences (excluding strings) and mappings (excluding our base types) have special meaning and + # cannot be used as default values unless wrapped inside Item. + + if root: + parent_section = root + + is_valid_config_root_declaration = ( + inspect.ismodule(declaration) + or + ( + isinstance(declaration, collections.Sequence) + and len(declaration) > 0 + and isinstance(declaration[0], tuple) + ) + or + ( + isinstance(declaration, collections.Mapping) + and len(declaration) > 0 + ) + ) + if not is_valid_config_root_declaration: + raise ValueError( + 'Config root declaration has to be a module, a non-empty sequence, or non-empty mapping, ' + 'got a {}'.format(type(declaration)), + ) + + if isinstance(declaration, (BaseItem, BaseSection)): + # Do not parse existing objects of our hierarchy + # TODO However, we should probably process raw sections! + # TODO Also, how would it be possible for user to legally create a new Section? + return declaration + + elif inspect.ismodule(declaration): + return parse_config_declaration(declaration.__dict__, parent_section=parent_section, root=root) + + elif isinstance(declaration, collections.Mapping): + + if len(declaration) == 0: + # Empty dictionary means an empty item + return parent_section.create_item(default=declaration) + + # Create a list of tuples so we can use the standard declaration parser below + return parse_config_declaration([x for x in declaration.items()], parent_section=parent_section, root=root) + + if isinstance(declaration, collections.Sequence) and not isinstance(declaration, six.string_types): + + if len(declaration) == 0 or not isinstance(declaration[0], tuple): + # Declaration of an item + return parent_section.create_item(default=declaration) + + # Pre-process all keys and discard private parts and separate out meta parts + clean_declaration = [] + meta = {} + + for k, v in declaration: + if k.startswith('_'): + continue + elif k.startswith('@'): + meta[k[1:]] = v + continue + clean_declaration.append((k, v)) + + if not clean_declaration or meta.get('type'): + # Must be an item + if not clean_declaration: + return parent_section.create_item(**meta) + else: + meta['default'] = dict(clean_declaration) + return parent_section.create_item(**meta) + + section = root or parent_section.create_section() + + for k, v in clean_declaration: + obj = parse_config_declaration(v, parent_section=section) + if obj.is_section: + section.add_section(k, obj) + else: + section.add_item(k, obj) + + return section + + # Declaration of an item + return parent_section.create_item(default=declaration) diff --git a/configmanager/managers.py b/configmanager/managers.py index 219e695..5de5c7c 100644 --- a/configmanager/managers.py +++ b/configmanager/managers.py @@ -1,5 +1,5 @@ +from .config_declaration_parser import parse_config_declaration from .meta import ConfigManagerSettings -from .parsers import ConfigDeclarationParser from .persistence import ConfigPersistenceAdapter, YamlReaderWriter, JsonReaderWriter, ConfigParserReaderWriter from .sections import Section @@ -91,6 +91,8 @@ class Config(Section): """ + is_config = True + def __init__(self, config_declaration=None, **configmanager_settings): if 'configmanager_settings' in configmanager_settings: if len(configmanager_settings) > 1: @@ -107,10 +109,8 @@ def __init__(self, config_declaration=None, **configmanager_settings): self._configmanager_yaml_adapter = None self._configmanager_click_extension = None - self._configmanager_process_config_declaration = ConfigDeclarationParser(section=self) - - if config_declaration: - self._configmanager_process_config_declaration(config_declaration) + if config_declaration is not None: + parse_config_declaration(config_declaration, root=self) self.load_user_app_config() diff --git a/configmanager/parsers.py b/configmanager/parsers.py deleted file mode 100644 index 6657109..0000000 --- a/configmanager/parsers.py +++ /dev/null @@ -1,108 +0,0 @@ -import copy -import inspect -import os.path -import types - -import collections -import six - -from .base import is_config_item, is_config_section - - -def is_config_declaration(obj): - return ( - isinstance(obj, (dict, types.ModuleType)) - or - inspect.isclass(obj) - ) - - -def get_file_adapter_name(filename): - adapter_lookup = { - '.json': 'json', - '.js': 'json', - '.ini': 'configparser', - '.yaml': 'yaml', - '.yml': 'yaml', - } - - _, ext = os.path.splitext(filename) - return adapter_lookup.get(ext.lower(), 'configparser') - - -class ConfigDeclarationParser(object): - def __init__(self, section): - assert section - assert hasattr(section, 'create_item') - assert hasattr(section, 'create_section') - self.section = section - - def __call__(self, config_decl, section=None): - if section is None: - section = self.section - - if isinstance(config_decl, dict): - keys_and_values = config_decl.items() - elif isinstance(config_decl, types.ModuleType): - keys_and_values = config_decl.__dict__.items() - elif inspect.isclass(config_decl): - keys_and_values = config_decl.__dict__.items() - elif isinstance(config_decl, (tuple, list)): - items = collections.OrderedDict() - for x in config_decl: - if is_config_item(x): - items[x.name] = x - elif isinstance(x, tuple): - items[x[0]] = x[1] - elif isinstance(x, six.string_types): - items[x] = section.create_item() - else: - raise TypeError('Unexpected {!r} {!r} in list of items for config declaration'.format(type(x), x)) - keys_and_values = items.items() - elif isinstance(config_decl, six.string_types): - getattr(section, get_file_adapter_name(config_decl)).load(config_decl, as_defaults=True) - keys_and_values = () - else: - raise TypeError('Unsupported config declaration type {!r}'.format(type(config_decl))) - - for k, v in keys_and_values: - if not isinstance(k, six.string_types): - raise TypeError('Config section and item names must be strings, got {}: {!r}'.format(type(k), k)) - - if isinstance(v, dict): - # - # Detect dictionaries with meta information: - # - could be items with partially data and partially meta information - # - could be items with just meta information - # - could be sections with meta information (not really supported yet) - # - - v_meta_keys = {x for x in v.keys() if x.startswith('@')} - - if v_meta_keys: - # Remove meta information from v dictionary - v_meta = {mk[1:]: v.pop(mk) for mk in v_meta_keys} - - # It is an item if it has @type specified or it consisted of just meta information - if v_meta.get('type') or len(v) == 0: - if len(v) > 0: - # Use the dictionary of the rest of the keys as the definition of default value - v_meta.setdefault('default', v) - section.add_item(k, self.section.create_item(**v_meta)) - continue - - if k.startswith('_'): - continue - elif k.startswith('@'): - # Meta attributes not supported on sections at the moment - continue - elif is_config_section(v): - section.add_section(k, v) - elif is_config_declaration(v): - section.add_section(k, self.__call__(v, section=self.section.create_section())) - elif is_config_item(v): - section.add_item(k, copy.deepcopy(v)) - else: - section.add_item(k, self.section.create_item(default=copy.deepcopy(v))) - - return section diff --git a/configmanager/persistence.py b/configmanager/persistence.py index 01f505e..892534f 100644 --- a/configmanager/persistence.py +++ b/configmanager/persistence.py @@ -212,6 +212,9 @@ def _load_config_from_config_parser(self, config, cp, as_defaults=False): # TODO Clean up the repetition here! + # TODO Shouldn't really use create_item and create_section methods here, + # TODO should use load_values(..., as_defaults=True) instead! + for option, value in cp.defaults().items(): if as_defaults: if option not in config: diff --git a/configmanager/sections.py b/configmanager/sections.py index 2d3862c..925b644 100644 --- a/configmanager/sections.py +++ b/configmanager/sections.py @@ -194,6 +194,10 @@ def add_section(self, alias, section): section._configmanager_section = self section._configmanager_section_alias = alias + # Must not mess around with settings of other Config instances. + if not section.is_config: + section._configmanager_settings = self._configmanager_settings + self.hooks.handle(Hooks.SECTION_ADDED_TO_SECTION, alias=alias, section=self, subject=section) def _get_str_path_separator(self, override=not_set): diff --git a/tests/test_config.py b/tests/test_config.py index 2e35d61..87f64cd 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -5,46 +5,6 @@ from configmanager import Config, Section, Item, Types, NotFound -def test_items_are_created_using_create_item_method(): - class CustomItem(Item): - pass - - class CustomConfig(Config): - def create_item(self, *args, **kwargs): - return CustomItem(*args, **kwargs) - - config = CustomConfig({ - 'a': {'b': {'c': {'d': 1, 'e': '2', 'f': True}}}, - 'g': False, - }) - - assert isinstance(config.g, CustomItem) - assert isinstance(config.a.b.c.d, CustomItem) - - -def test_config_create_section_creates_instance_of_section_class(): - class ExtendedConfig(Config): - pass - - config = ExtendedConfig({ - 'uploads': { - 'db': { - 'user': 'root' - }, - 'api': { - 'port': 8000, - 'extensions': {}, - }, - } - }) - - assert isinstance(config, ExtendedConfig) - assert isinstance(config.uploads, Section) - assert isinstance(config.uploads.db, Section) - assert isinstance(config.uploads.db.user, Item) - assert isinstance(config.uploads.api.extensions, Section) - - def test_reset_resets_values_to_defaults(): config = Config({ 'x': Item(type=int), diff --git a/tests/test_config_declaration_parser.py b/tests/test_config_declaration_parser.py index 5daabbc..901da25 100644 --- a/tests/test_config_declaration_parser.py +++ b/tests/test_config_declaration_parser.py @@ -4,32 +4,10 @@ from configmanager.items import Item from configmanager import Config, Types +from configmanager.config_declaration_parser import parse_config_declaration from configmanager.sections import Section -@pytest.fixture -def app_config_cls_example(): - class DeepConfig: - question = 'Why would you want a config this deep?' - - class UploadsConfig: - enabled = True - threads = 1 - type_ = Item(name='type', default=None) - - class DownloadsConfig: - content_type = 'text/plain' - deep = DeepConfig - threads = Item(type=int) - - class AppConfig: - uploads = UploadsConfig - downloads = DownloadsConfig - greeting = 'Hello, world!' - - return AppConfig - - @pytest.fixture def app_config_dict_example(): return { @@ -72,67 +50,72 @@ def app_config_module_example(): return app_config -@pytest.fixture -def app_config_mixed_example(): - class DeepConfig: - question = 'Why would you want a config this deep?' - - uploads_module = types.ModuleType('uploads') - uploads_module.enabled = True - uploads_module.threads = 1 - uploads_module.type_ = Item(name='type', default=None) - - downloads_dict = { - 'content_type': 'text/plain', - 'deep': DeepConfig, - 'threads': Item(type=int), - } - - class AppConfig: - uploads = uploads_module - downloads = downloads_dict - greeting = 'Hello, world!' - - return AppConfig - - -def test_class_based_config_declaration(app_config_cls_example): - tree = Config(app_config_cls_example) - - assert tree['uploads'] - assert tree['uploads']['enabled'].name == 'enabled' - assert tree['uploads']['type'].name == 'type' - assert tree['uploads']['type'].value is None - - assert tree['downloads'] - assert tree['downloads']['deep'] - assert tree['downloads']['deep']['question'] - assert tree['downloads']['deep']['question'].value +def test_primitive_value_is_a_declaration_of_an_item(): + config = Config(( + ('enabled', True), + ('threads', 5), + )) - assert tree['downloads']['threads'].name == 'threads' - assert tree['greeting'].name == 'greeting' + assert config.enabled.is_item + assert config.enabled.default is True + assert config.enabled.name == 'enabled' - assert 'deep' not in tree - assert 'question' not in tree - assert 'enabled' not in tree + assert config.threads.is_item + assert config.threads.default == 5 + assert config.threads.name == 'threads' -def test_dict_based_config_declaration(app_config_dict_example, app_config_cls_example): - dict_tree = Config(app_config_dict_example) - cls_tree = Config(app_config_cls_example) - assert dict_tree.dump_values() == cls_tree.dump_values() +def test_lists_of_tuples_declare_sections(): + config = Config() + parse_config_declaration([ + ('uploads', [ + ('enabled', True), + ('threads', 5), + ('db', [ + ('user', 'root'), + ('password', 'secret'), + ]), + ]), + ], root=config) + + assert config.uploads.is_section + assert config.uploads.enabled.is_item + assert config.uploads.enabled.default is True + assert config.uploads.threads.is_item + assert config.uploads.db.is_section + assert config.uploads.db.user.is_item + assert config.uploads.db.user.default == 'root' + assert config.uploads.db.password.is_item + + +def test_non_empty_dictionaries_declare_sections(): + config = Config() + parse_config_declaration({ + 'uploads': { + 'enabled': True, + 'threads': 5, + 'db': { + 'user': 'root', + 'password': 'secret', + }, + }, + }, root=config) -def test_module_based_config_declaration(app_config_module_example, app_config_cls_example): - module_tree = Config(app_config_module_example) - cls_tree = Config(app_config_cls_example) - assert module_tree.dump_values() == cls_tree.dump_values() + assert config.uploads.is_section + assert config.uploads.enabled.is_item + assert config.uploads.enabled.default is True + assert config.uploads.threads.is_item + assert config.uploads.db.is_section + assert config.uploads.db.user.is_item + assert config.uploads.db.user.default == 'root' + assert config.uploads.db.password.is_item -def test_mixed_config_declaration(app_config_mixed_example, app_config_cls_example): - mixed_tree = Config(app_config_mixed_example) - cls_tree = Config(app_config_cls_example) - assert mixed_tree.dump_values() == cls_tree.dump_values() +def test_dict_and_module_based_config_declaration(app_config_dict_example, app_config_module_example): + dict_config = Config(app_config_dict_example) + module_config = Config(app_config_module_example) + assert dict_config.dump_values(with_defaults=True) == module_config.dump_values(with_defaults=True) def test_default_value_is_deep_copied(): @@ -145,77 +128,41 @@ def test_default_value_is_deep_copied(): assert config['items'].value == [1, 2, 3, 4] -def test_config_declaration_can_be_a_list_of_items_or_two_tuples(): - config = Config([ - ('enabled', True), - ('threads', 5), - Item('greeting'), - ('db', Config({'user': 'root'})) - ]) - assert list(path for path, _ in config.iter_items(recursive=True)) == [('enabled',), ('threads',), ('greeting',), ('db', 'user')] - - -def test_declaration_can_be_a_list_of_field_names(): +def test_can_use_list_notation_to_declare_ordered_config_tree(): config = Config([ - 'enabled', 'threads', 'greeting', 'tmp_dir', - ('db', Config(['user', 'host', 'password', 'name'])) + ('uploads', [ + ('enabled', False), + ('db', [ + ('user', 'root'), + ]) + ]) ]) - assert config.enabled - assert config.threads - assert config.greeting - assert config.tmp_dir - assert config.db.user - assert config.db.host - assert config.db.password - assert config.db.name - - assert not config.enabled.has_value - - -def test_declaration_cannot_be_a_list_of_other_things(): - with pytest.raises(TypeError): - Config(['enabled', True]) - - with pytest.raises(TypeError): - Config([True, 5]) + assert config.uploads.enabled.value is False + assert config.uploads.db.user.value == 'root' - with pytest.raises(TypeError): - Config([True, 5, 0.0]) - - -def test_declaration_can_be_a_filename(tmpdir): - config = Config([ - ('uploads', Config([ - ('enabled', True), - ('db', Config([ - ('user', 'root'), - ('password', 'secret'), - ])), - ('threads', 5), - ])) - ]) - json_path = tmpdir.join('uploads.json').strpath - ini_path = tmpdir.join('uploads.ini').strpath - yaml_path = tmpdir.join('uploads.yaml').strpath +def test_config_root_declaration_cannot_be_a_primitive_value(tmpdir): + # + with pytest.raises(ValueError): + Config(5) - config.json.dump(json_path, with_defaults=True) - config.yaml.dump(yaml_path, with_defaults=True) + with pytest.raises(ValueError): + Config(True) - c2 = Config(json_path) - assert c2.dump_values(with_defaults=True) == config.dump_values(with_defaults=True) + with pytest.raises(ValueError): + Config([]) - c3 = Config(yaml_path) - assert c3.dump_values(with_defaults=True) == config.dump_values(with_defaults=True) + with pytest.raises(ValueError): + Config({}) - # For configparser test 2 levels only: - config.uploads.configparser.dump(ini_path, with_defaults=True) + with pytest.raises(ValueError): + Config('haha') - c4 = Config(ini_path) - print(c4.configparser.dumps(with_defaults=True)) - print(config.uploads.configparser.dumps(with_defaults=True)) - assert c4.configparser.dumps(with_defaults=True) == config.uploads.configparser.dumps(with_defaults=True) + # Also, no more filenames + json_filename = tmpdir.join('uploads.json').strpath + with pytest.raises(ValueError): + Config(json_filename) def test_dict_with_all_keys_prefixed_with_at_symbol_is_treated_as_item_meta(): @@ -300,7 +247,7 @@ def test_accepts_configmanager_settings_which_are_passed_to_all_subsections(): configmanager_settings = { 'message': 'everyone should know this', } - config1 = Config({}, configmanager_settings=configmanager_settings) + config1 = Config(configmanager_settings=configmanager_settings) assert config1._configmanager_settings.message == 'everyone should know this' config2 = Config({'greeting': 'Hello'}, configmanager_settings=configmanager_settings) @@ -311,3 +258,92 @@ def test_accepts_configmanager_settings_which_are_passed_to_all_subsections(): assert config3.db._configmanager_settings.message == 'everyone should know this' assert config3.db._configmanager_settings is config3._configmanager_settings + + +def test_empty_list_is_an_item_with_list_type(): + config = Config({ + 'tags': [], + 'uploads': { + 'tmp_dirs': [], + }, + }) + + assert config.tags.is_item + assert config.tags.type == Types.list + assert config.tags.default == [] + + assert config.uploads.tmp_dirs.is_item + assert config.uploads.tmp_dirs.type == Types.list + assert config.uploads.tmp_dirs.default == [] + + +def test_list_of_strings_is_an_item_with_list_type(): + config = Config({ + 'tmp_dirs': ['tmp'], + 'target_dirs': ['target1', 'target2'], + 'other_dirs': ['other1', 'other2', 'other3'], + 'uploads': { + 'dirs': ['dir1', 'dir2'], + } + }) + + assert config.tmp_dirs.default == ['tmp'] + assert config.target_dirs.default == ['target1', 'target2'] + assert config.other_dirs.default == ['other1', 'other2', 'other3'] + assert config.uploads.dirs.default == ['dir1', 'dir2'] + + +def test_empty_dict_is_an_item_with_dict_type(): + config = Config({ + 'uploads': {} + }) + + assert config.uploads.is_item + assert config.uploads.default == {} + assert config.uploads.type == Types.dict + + +def test_can_declare_empty_section_and_it_gets_updated_with_references_to_config(): + config = Config({ + 'uploads': Section(), + 'api': { + 'db': Section(), + }, + }) + + assert config.uploads.is_section + assert config.uploads.section is config + assert config.uploads._configmanager_settings is config._configmanager_settings + + assert config.api.db.is_section + assert config.api.db.section is config.api + assert config.api.db._configmanager_settings is config._configmanager_settings + + assert config.api.section is config + assert config.api._configmanager_settings is config._configmanager_settings + + +def test_can_reassign_a_section_of_one_config_to_another_and_all_its_subsections_get_updated(): + config1 = Config({ + 'uploads': { + 'api': { + 'db': Section() + }, + }, + }) + + config2 = Config({ + 'uploads': config1.uploads + }) + + assert config1._configmanager_settings is not config2._configmanager_settings + + assert config2.uploads.section is config2 + assert config2.uploads._configmanager_settings is config2._configmanager_settings + + assert config2.uploads.api.section is config2.uploads + + # TODO Think more about this + # Note that config1 is now left in a weird state and we can't be bothered about + # TODO Probably should make this illegal. But can we detect that someone tries to reassign + # TODO a section from another tree? diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 7a43f62..d6e3488 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -1,6 +1,6 @@ import pytest -from configmanager import Config, NotFound +from configmanager import Config, NotFound, Section from configmanager.hooks import Hooks from configmanager.utils import not_set @@ -26,7 +26,7 @@ def test_not_found_hook(): calls = [] config = Config({ - 'uploads': {} + 'uploads': Section() }) @config.hooks.not_found diff --git a/tests/test_iterators.py b/tests/test_iterators.py index c818ce6..ff912ac 100644 --- a/tests/test_iterators.py +++ b/tests/test_iterators.py @@ -53,8 +53,8 @@ def c4(): @pytest.fixture def c5(): return Config([ - 'greeting', - 'tmp_dir', + ('greeting', ''), + ('tmp_dir', ''), ]) diff --git a/tests/test_v1.py b/tests/test_v1.py index b82d495..2a5353b 100644 --- a/tests/test_v1.py +++ b/tests/test_v1.py @@ -84,29 +84,18 @@ def test_nested_config(): 'port': 8080, }) - # Also, it can be a Python module (actual module instance), -- not shown here - # or a class: - class ClientConfig: - timeout = 10 - # # All these sections can be combined into one config: # config = Config({ 'db': db_config, 'server': server_config, - 'client': ClientConfig, # a class, not an instance 'greeting': 'Hello', # and you can have plain config items next to sections }) # You can load values - assert config.client.timeout.value == 10 assert config.greeting.value == 'Hello' - # You can change values and they will be converted to the right type if possible - config.client.timeout.value = '20' - assert config.client.timeout.value == 20 - # Your original declarations are safe -- db_config dictionary won't be changed config.db.user.value = 'root' assert config.db.user.value == 'root' @@ -150,10 +139,8 @@ class ClientConfig: assert config.db.user.value == 'admin' # Or you can reset all configuration and you can make sure all values match defaults - assert config.client.timeout.value == 20 assert not config.is_default config.reset() - assert config.client.timeout.value == 10 assert config.is_default