From b86f334b3ca7e1e1fb978df1249ba649bd96ddd2 Mon Sep 17 00:00:00 2001 From: Evaline Ju <69598118+evaline-ju@users.noreply.github.com> Date: Mon, 24 Apr 2023 14:34:19 -0600 Subject: [PATCH 1/7] :white_check_mark::construction: Immutable dict tests Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com> --- test/test_attribute_access_dict.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/test_attribute_access_dict.py b/test/test_attribute_access_dict.py index ab0f00f..897b410 100644 --- a/test/test_attribute_access_dict.py +++ b/test/test_attribute_access_dict.py @@ -213,3 +213,21 @@ def test_builtin_method(self): self.assertNotEqual(aad.update, None) self.assertNotIn('update', aad) + + def test_immutable_flat_access_dict(self): + '''Test that frozen flat access dict cannot be changed + ''' + flat_dict = aconfig.AttributeAccessDict(fixtures.GOOD_FLAT_DICT, frozen=True) + self.assertIsInstance(flat_dict, aconfig.AttributeAccessDict) + + with self.assertRaises(TypeError): + flat_dict['str_key'] = 'new_key' + + def test_immutable_nested_access_dict(self): + '''Test that frozen nested access dict cannot be changed + ''' + flat_dict = aconfig.AttributeAccessDict(fixtures.GOOD_NESTED_DICT, frozen=True) + self.assertIsInstance(flat_dict, aconfig.AttributeAccessDict) + + with self.assertRaises(TypeError): + flat_dict['key2']['key4'] = 'new_key' From 67f67b8d1bbca549a164a06d3640ca521d20dc86 Mon Sep 17 00:00:00 2001 From: Evaline Ju <69598118+evaline-ju@users.noreply.github.com> Date: Mon, 24 Apr 2023 14:34:43 -0600 Subject: [PATCH 2/7] :white_check_mark: yaml full load Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com> --- test/test_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_config.py b/test/test_config.py index 717f51c..c3d9947 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -231,6 +231,6 @@ def test_yaml_dump(self): assert yaml_dump == yaml_safe_dump # Load both ways - yaml_loaded = yaml.load(yaml_dump) + yaml_loaded = yaml.full_load(yaml_dump) yaml_safe_loaded = yaml.safe_load(yaml_dump) assert yaml_loaded == yaml_safe_loaded From fc6337ad606cdf2c70400ff7f97ba25444bab17a Mon Sep 17 00:00:00 2001 From: Joe Runde Date: Mon, 24 Apr 2023 14:58:19 -0600 Subject: [PATCH 3/7] :sparkles: add immutable config Signed-off-by: Joe Runde --- aconfig/__init__.py | 2 +- aconfig/aconfig.py | 41 +++++++++++++++++++++++++----- test/test_attribute_access_dict.py | 8 +++--- test/test_config.py | 27 ++++++++++++++++++++ 4 files changed, 66 insertions(+), 12 deletions(-) diff --git a/aconfig/__init__.py b/aconfig/__init__.py index f29d9d2..b61a412 100644 --- a/aconfig/__init__.py +++ b/aconfig/__init__.py @@ -8,4 +8,4 @@ '''Enable calls to desired components of package. ''' -from .aconfig import AttributeAccessDict, Config +from .aconfig import AttributeAccessDict, Config, ImmutableAttributeAccessDict, ImmutableConfig diff --git a/aconfig/aconfig.py b/aconfig/aconfig.py index 94216c7..c81a02b 100644 --- a/aconfig/aconfig.py +++ b/aconfig/aconfig.py @@ -41,19 +41,19 @@ def __init__(self, input_map): # recursively instantiate sub-dicts for key, value in copied_map.items(): - copied_map[key] = AttributeAccessDict._make_attribute_access_dict(value) + copied_map[key] = self.__class__._make_attribute_access_dict(value) # make it accessible like native Python dict super().__init__(**copied_map) - @staticmethod - def _make_attribute_access_dict(value): - if isinstance(value, AttributeAccessDict): + @classmethod + def _make_attribute_access_dict(cls, value): + if isinstance(value, cls): return value elif isinstance(value, dict): - return AttributeAccessDict(value) + return cls(value) elif isinstance(value, list): - return [AttributeAccessDict._make_attribute_access_dict(v) for v in value] + return [cls._make_attribute_access_dict(v) for v in value] else: return value @@ -83,7 +83,28 @@ def __deepcopy__(self, memo): '''This enables deepcopy to successfully copy a Config object, despite the default value semantics ''' - return AttributeAccessDict(copy.deepcopy(dict(self))) + return self.__class__(copy.deepcopy(dict(self))) + + +class ImmutableAttributeAccessDict(AttributeAccessDict): + + def __init__(self, input_map, *_): + assert isinstance(input_map, dict), \ + '`input_map` argument should be of type dict, but found type: <{0}>'.format( + type(input_map)) + # 🌶️🌶️🌶️: we explicitly cast back down to `dict` for the immutable case + # If we were to build an immutable dict from the top-down, that would + # obviously fail. + input_map = dict(input_map) + # Invoke the AttributeAccessDict initializer + super().__init__(input_map) + + def __setitem__(self, key, value): + raise TypeError("Nope") + + def __setattr__(self, key, value): + raise TypeError("Nope") + class Config(AttributeAccessDict): '''Config which holds the configurations at the given config location. @@ -293,6 +314,12 @@ def _env_var_from_key(self, config_key): return re.sub(self._search_pattern, '_', config_key.upper()) +class ImmutableConfig(ImmutableAttributeAccessDict, Config): + def __init__(self, config, override_env_vars=True): + assert isinstance(config, dict) + super().__init__(config, override_env_vars) + + ## yaml representation safe ######################################################################## yaml.add_representer(AttributeAccessDict, SafeRepresenter.represent_dict) diff --git a/test/test_attribute_access_dict.py b/test/test_attribute_access_dict.py index 897b410..ffd0932 100644 --- a/test/test_attribute_access_dict.py +++ b/test/test_attribute_access_dict.py @@ -215,18 +215,18 @@ def test_builtin_method(self): self.assertNotIn('update', aad) def test_immutable_flat_access_dict(self): - '''Test that frozen flat access dict cannot be changed + '''Test that immutable flat dict cannot be changed ''' - flat_dict = aconfig.AttributeAccessDict(fixtures.GOOD_FLAT_DICT, frozen=True) + flat_dict = aconfig.ImmutableAttributeAccessDict(fixtures.GOOD_FLAT_DICT) self.assertIsInstance(flat_dict, aconfig.AttributeAccessDict) with self.assertRaises(TypeError): flat_dict['str_key'] = 'new_key' def test_immutable_nested_access_dict(self): - '''Test that frozen nested access dict cannot be changed + '''Test that immutable nested dict cannot be changed ''' - flat_dict = aconfig.AttributeAccessDict(fixtures.GOOD_NESTED_DICT, frozen=True) + flat_dict = aconfig.ImmutableAttributeAccessDict(fixtures.GOOD_NESTED_DICT) self.assertIsInstance(flat_dict, aconfig.AttributeAccessDict) with self.assertRaises(TypeError): diff --git a/test/test_config.py b/test/test_config.py index c3d9947..b94c415 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -234,3 +234,30 @@ def test_yaml_dump(self): yaml_loaded = yaml.full_load(yaml_dump) yaml_safe_loaded = yaml.safe_load(yaml_dump) assert yaml_loaded == yaml_safe_loaded + + def test_immutable_config(self): + cfg = aconfig.ImmutableConfig({'a': {'b': [{'c': 1}]}}) + self.assertEqual(cfg.a.b[0].c, 1) + + with self.assertRaises(TypeError): + cfg.a.b[0].c = 2 + with self.assertRaises(TypeError): + cfg.a.b = [1, 2, 3] + with self.assertRaises(TypeError): + cfg.a = 1 + + def test_immutable_config_with_env_overrides(self): + # set an environment + os.environ['KEY1'] = '12345678' + cfg = aconfig.ImmutableConfig({"key1": 1, "key2": 2}, override_env_vars=True) + + assert cfg.key2 == 2 + assert cfg.key1 == 12345678 + with self.assertRaises(TypeError): + cfg.key1 = 1 + + def test_immutable_config_from_mutable_config(self): + cfg = aconfig.Config({'a': {'b': [{'c': 1}]}}) + immutable_config = aconfig.ImmutableConfig(cfg) + + assert cfg == immutable_config From 18c985cc8834f15e821407d9149865769eeb56a7 Mon Sep 17 00:00:00 2001 From: Joe Runde Date: Mon, 24 Apr 2023 15:12:08 -0600 Subject: [PATCH 4/7] :memo: Add docstrings, more tests Signed-off-by: Joe Runde --- aconfig/aconfig.py | 16 ++++++++++++++-- test/test_config.py | 6 ++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/aconfig/aconfig.py b/aconfig/aconfig.py index c81a02b..67b18ea 100644 --- a/aconfig/aconfig.py +++ b/aconfig/aconfig.py @@ -48,6 +48,9 @@ def __init__(self, input_map): @classmethod def _make_attribute_access_dict(cls, value): + """Recursively walk down any `dict`s or `list`s and build attribute access dicts + 🌶️🌶️🌶️: This is a classmethod so that inheritance is respected. + """ if isinstance(value, cls): return value elif isinstance(value, dict): @@ -87,8 +90,15 @@ def __deepcopy__(self, memo): class ImmutableAttributeAccessDict(AttributeAccessDict): + """This class subclasses AttributeAccessDict and removes the setters, + to allow the creation of immutable dicts. + + Using inheritance this way allows the dicts to be recursively created via + AttributeAccessDict, while maintaining nested immutability. + """ def __init__(self, input_map, *_): + """See :func:`~aconfig.aconfig.AttributeAccessDict.__init__`""" assert isinstance(input_map, dict), \ '`input_map` argument should be of type dict, but found type: <{0}>'.format( type(input_map)) @@ -100,10 +110,10 @@ def __init__(self, input_map, *_): super().__init__(input_map) def __setitem__(self, key, value): - raise TypeError("Nope") + raise TypeError("ImmutableAttributeAccessDict does not support item assignment") def __setattr__(self, key, value): - raise TypeError("Nope") + raise TypeError("ImmutableAttributeAccessDict does not support item assignment") class Config(AttributeAccessDict): @@ -315,7 +325,9 @@ def _env_var_from_key(self, config_key): class ImmutableConfig(ImmutableAttributeAccessDict, Config): + """This class is the Immutable version of Config""" def __init__(self, config, override_env_vars=True): + """See :func:`~aconfig.aconfig.Config.__init__`""" assert isinstance(config, dict) super().__init__(config, override_env_vars) diff --git a/test/test_config.py b/test/test_config.py index b94c415..ad03afd 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -221,6 +221,12 @@ def test_deepcopy(self): self.assertEqual(cfg.a.b[0].c, 1) self.assertEqual(cfg_copy.a.b[0].c, 2) + immutable_cfg = aconfig.ImmutableConfig(cfg) + immutable_cfg_copy = copy.deepcopy(immutable_cfg) + self.assertIsInstance(immutable_cfg_copy, aconfig.ImmutableConfig) + self.assertEqual(immutable_cfg_copy, immutable_cfg) + self.assertIsNot(immutable_cfg_copy, immutable_cfg) + def test_yaml_dump(self): '''Test yaml.dump(config) works''' loaded_yaml = aconfig.Config.from_yaml(fixtures.GOOD_CONFIG_LOCATION) From 8641b4daeab53b4c61147c62855d4dead5683d55 Mon Sep 17 00:00:00 2001 From: Evaline Ju <69598118+evaline-ju@users.noreply.github.com> Date: Mon, 24 Apr 2023 16:31:42 -0600 Subject: [PATCH 5/7] :goal_net: Raise attribute error Co-authored-by: Gabe Goodhart Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com> --- aconfig/aconfig.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aconfig/aconfig.py b/aconfig/aconfig.py index 67b18ea..0811c45 100644 --- a/aconfig/aconfig.py +++ b/aconfig/aconfig.py @@ -113,7 +113,7 @@ def __setitem__(self, key, value): raise TypeError("ImmutableAttributeAccessDict does not support item assignment") def __setattr__(self, key, value): - raise TypeError("ImmutableAttributeAccessDict does not support item assignment") + raise AttributeError("ImmutableAttributeAccessDict does not support attribute assignment") class Config(AttributeAccessDict): From 01a181120e455924ef81a743e6bff2d44a0230f3 Mon Sep 17 00:00:00 2001 From: Evaline Ju <69598118+evaline-ju@users.noreply.github.com> Date: Mon, 24 Apr 2023 16:41:16 -0600 Subject: [PATCH 6/7] :goal_net::white_check_mark: TypeError for non-dict config Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com> --- aconfig/aconfig.py | 17 +++++++++-------- test/test_attribute_access_dict.py | 8 ++++---- test/test_config.py | 8 ++++---- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/aconfig/aconfig.py b/aconfig/aconfig.py index 0811c45..68b02d9 100644 --- a/aconfig/aconfig.py +++ b/aconfig/aconfig.py @@ -32,9 +32,9 @@ def __init__(self, input_map): overrides dict's methods to enable this. Can be modified later on and keep the same behavior. ''' - assert isinstance(input_map, dict), \ - '`input_map` argument should be of type dict, but found type: <{0}>'.format( - type(input_map)) + if not isinstance(input_map, dict): + raise TypeError('`input_map` argument should be of type dict, but found type: <{0}>'.format( + type(input_map))) # copy so as not to modify passed in dictionary copied_map = copy.deepcopy(input_map) @@ -99,9 +99,9 @@ class ImmutableAttributeAccessDict(AttributeAccessDict): def __init__(self, input_map, *_): """See :func:`~aconfig.aconfig.AttributeAccessDict.__init__`""" - assert isinstance(input_map, dict), \ - '`input_map` argument should be of type dict, but found type: <{0}>'.format( - type(input_map)) + if not isinstance(input_map, dict): + raise TypeError('`input_map` argument should be of type dict, but found type: <{0}>'.format( + type(input_map))) # 🌶️🌶️🌶️: we explicitly cast back down to `dict` for the immutable case # If we were to build an immutable dict from the top-down, that would # obviously fail. @@ -110,7 +110,7 @@ def __init__(self, input_map, *_): super().__init__(input_map) def __setitem__(self, key, value): - raise TypeError("ImmutableAttributeAccessDict does not support item assignment") + raise AttributeError("ImmutableAttributeAccessDict does not support attribute assignment") def __setattr__(self, key, value): raise AttributeError("ImmutableAttributeAccessDict does not support attribute assignment") @@ -328,7 +328,8 @@ class ImmutableConfig(ImmutableAttributeAccessDict, Config): """This class is the Immutable version of Config""" def __init__(self, config, override_env_vars=True): """See :func:`~aconfig.aconfig.Config.__init__`""" - assert isinstance(config, dict) + if not isinstance(config, dict): + raise TypeError("config must be a dict") super().__init__(config, override_env_vars) diff --git a/test/test_attribute_access_dict.py b/test/test_attribute_access_dict.py index ffd0932..3a5b100 100644 --- a/test/test_attribute_access_dict.py +++ b/test/test_attribute_access_dict.py @@ -33,11 +33,11 @@ def test__init__pass(self): def test__init__fail(self): '''Test that initialization will fail with invalid dict's. ''' - with self.assertRaises(AssertionError) as ex: + with self.assertRaises(TypeError) as ex: bad_dict = aconfig.AttributeAccessDict(['a', 'list', 'of', 'strings']) raised_exception = ex.exception - self.assertIsInstance(raised_exception, AssertionError) + self.assertIsInstance(raised_exception, TypeError) # make sure bad_dict was NOT initialized self.assertEqual(getattr(locals(), 'bad_dict', None), None) @@ -220,7 +220,7 @@ def test_immutable_flat_access_dict(self): flat_dict = aconfig.ImmutableAttributeAccessDict(fixtures.GOOD_FLAT_DICT) self.assertIsInstance(flat_dict, aconfig.AttributeAccessDict) - with self.assertRaises(TypeError): + with self.assertRaises(AttributeError): flat_dict['str_key'] = 'new_key' def test_immutable_nested_access_dict(self): @@ -229,5 +229,5 @@ def test_immutable_nested_access_dict(self): flat_dict = aconfig.ImmutableAttributeAccessDict(fixtures.GOOD_NESTED_DICT) self.assertIsInstance(flat_dict, aconfig.AttributeAccessDict) - with self.assertRaises(TypeError): + with self.assertRaises(AttributeError): flat_dict['key2']['key4'] = 'new_key' diff --git a/test/test_config.py b/test/test_config.py index ad03afd..fcd113a 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -245,11 +245,11 @@ def test_immutable_config(self): cfg = aconfig.ImmutableConfig({'a': {'b': [{'c': 1}]}}) self.assertEqual(cfg.a.b[0].c, 1) - with self.assertRaises(TypeError): + with self.assertRaises(AttributeError): cfg.a.b[0].c = 2 - with self.assertRaises(TypeError): + with self.assertRaises(AttributeError): cfg.a.b = [1, 2, 3] - with self.assertRaises(TypeError): + with self.assertRaises(AttributeError): cfg.a = 1 def test_immutable_config_with_env_overrides(self): @@ -259,7 +259,7 @@ def test_immutable_config_with_env_overrides(self): assert cfg.key2 == 2 assert cfg.key1 == 12345678 - with self.assertRaises(TypeError): + with self.assertRaises(AttributeError): cfg.key1 = 1 def test_immutable_config_from_mutable_config(self): From b0af872a40eb3a51ab42780a5d6fac7951b0999e Mon Sep 17 00:00:00 2001 From: Evaline Ju <69598118+evaline-ju@users.noreply.github.com> Date: Tue, 25 Apr 2023 10:38:07 -0600 Subject: [PATCH 7/7] :goal_net::white_check_mark: Type error for setitem Signed-off-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com> --- aconfig/aconfig.py | 2 +- test/test_attribute_access_dict.py | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/aconfig/aconfig.py b/aconfig/aconfig.py index 68b02d9..ae021da 100644 --- a/aconfig/aconfig.py +++ b/aconfig/aconfig.py @@ -110,7 +110,7 @@ def __init__(self, input_map, *_): super().__init__(input_map) def __setitem__(self, key, value): - raise AttributeError("ImmutableAttributeAccessDict does not support attribute assignment") + raise TypeError("ImmutableAttributeAccessDict does not support item assignment") def __setattr__(self, key, value): raise AttributeError("ImmutableAttributeAccessDict does not support attribute assignment") diff --git a/test/test_attribute_access_dict.py b/test/test_attribute_access_dict.py index 3a5b100..1d95500 100644 --- a/test/test_attribute_access_dict.py +++ b/test/test_attribute_access_dict.py @@ -220,7 +220,7 @@ def test_immutable_flat_access_dict(self): flat_dict = aconfig.ImmutableAttributeAccessDict(fixtures.GOOD_FLAT_DICT) self.assertIsInstance(flat_dict, aconfig.AttributeAccessDict) - with self.assertRaises(AttributeError): + with self.assertRaises(TypeError): flat_dict['str_key'] = 'new_key' def test_immutable_nested_access_dict(self): @@ -229,5 +229,14 @@ def test_immutable_nested_access_dict(self): flat_dict = aconfig.ImmutableAttributeAccessDict(fixtures.GOOD_NESTED_DICT) self.assertIsInstance(flat_dict, aconfig.AttributeAccessDict) - with self.assertRaises(AttributeError): + with self.assertRaises(TypeError): flat_dict['key2']['key4'] = 'new_key' + + def test_immutable_dict_attr(self): + '''Test that immutable dict cannot be changed via attribute + ''' + flat_dict = aconfig.ImmutableAttributeAccessDict(fixtures.GOOD_FLAT_DICT) + self.assertIsInstance(flat_dict, aconfig.AttributeAccessDict) + + with self.assertRaises(AttributeError): + flat_dict.str_key = 'new_key'