Repository: bradleygolden/configureme

Issue:
Rename still uses original env value when it shouldn't
Say "ENV_FOO=BAR" is set in the environment. `config.from_envar("ENV_FOO", rename="FOO")` should work like this when used throughout a project:

```python
config["FOO"]  # returns "BAR"
config["ENV_FOOR"]  # raises KeyError
```
Right now however, both of these work:

```
config["FOO"]  # returns "BAR"
config["ENV_FOO"]  # returns "BAR"
```

This is a problem!


Discussion:

Patch:
diff --git a/.vscode/settings.json b/.vscode/settings.json
deleted file mode 100644
index 184c2da..0000000
--- a/.vscode/settings.json
+++ /dev/null
@@ -1,18 +0,0 @@
-{
-    "python.formatting.provider": "black",
-    "python.formatting.blackArgs": ["--line-length", "88"],
-    "[python]": {
-        "editor.formatOnPaste": false,
-        "editor.codeActionsOnSave": {
-            "source.organizeImports": true
-        }
-    },
-    "python.sortImports.args": [
-        "-rc",
-        "--atomic"
-    ],
-    "python.linting.pylintEnabled": false,
-    "python.linting.flake8Enabled": true,
-    "python.linting.enabled": true,
-    "python.linting.mypyEnabled": false,
-}
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c82ef20..d118de2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.1.2a0] - 2018-11-03
+
+## Fixed
+
+- Renaming issue where the original name of variable that was renamed could still be accessed.
+
+
 ## [0.1.1a0] - 2018-10-28
 
 ### Added
diff --git a/configureme/__version__.py b/configureme/__version__.py
index 77f1e31..4b45137 100644
--- a/configureme/__version__.py
+++ b/configureme/__version__.py
@@ -9,7 +9,7 @@
 #                        |___/
 
 
-VERSION = (0, 1, 1)
+VERSION = (0, 1, 2)
 PRE = "a0"
 
 __version__ = ".".join(map(str, VERSION)) + PRE
diff --git a/configureme/core.py b/configureme/core.py
index f5f80e4..c1710a8 100644
--- a/configureme/core.py
+++ b/configureme/core.py
@@ -2,69 +2,102 @@
 import os
 import warnings
 
-from .helpers import dotenv_to_dict, str_to_py
+from .helpers import dotenv_to_dict, envar_exists, get_envar
 
 
-class Config(dict):
+class ConfigVar:
+    def __init__(self, name, value=None, alias=None, default=None, use_env=False):
+        self.name = name
+        self._value = value
+        self.alias = alias
+        self.default = default
+        self.use_env = use_env
+
+    @property
+    def value(self):
+        if self.use_env:
+            if envar_exists(self.name):
+                return get_envar(self.name)
+            elif not envar_exists(self.name) and self._value:
+                return self._value
+            elif not envar_exists(self.name) and self.default:
+                return self.default
+            elif not envar_exists(self.name):
+                raise KeyError(f"{self.name} is not a valid value.")
+        return self._value
+
+    @value.setter
+    def value(self, value):
+        self._value = value
+
+    def is_renamed(self):
+        return self.alias is not None
+
+
+class ConfigVars:
     def __init__(self):
-        self._aliases = dict()
-        self._defaults = dict()
-        self.envs = list()
+        self.store = {}
 
-    def __getitem__(self, key):
-        rv = None
-        # environment variables always take precedence
-        rv = self._env_value(key)
-        if rv is not None:
-            return rv
-        # no env variable value exists, use the config value if it exists
-        if key in self:
-            return self.get(key, None)
-        # no config value exists, use the default value if it exists
-        if key in self._defaults:
-            return self._defaults[key]
-        # no default value for the key, use the default value of its alias if it exists
-        if key in self._aliases:
-            key_alias = self._aliases[key]
-            if key_alias in self._defaults:
-                return self._defaults[key_alias]
-        raise ValueError(
-            f"Variable {key} value is not set as an environment variable or config value, and does not have a default value."
-        )
-
-    def _env_value(self, key: str):
-        rv = None
-        try:
-            env_value = os.environ[key]
-            rv = str_to_py(env_value)
-        except KeyError:
-            pass
+    def add(self, name, **kwargs):
+        if self.exists(name):
+            for key, value in kwargs.items():
+                if value:
+                    setattr(self.store[name], key, value)
+        else:
+            self.store[name] = ConfigVar(name, **kwargs)
+
+    def get(self, name):
         try:
-            env_value = os.environ[self._aliases[key]]
-            rv = str_to_py(env_value)
+            return self.store[name]
         except KeyError:
-            pass
+            for _, var in self.store.items():
+                if var.is_renamed:
+                    return var
+
+    def exists(self, name):
+        return name in self.store.keys()
+
+    def to_dict(self):
+        rv = {}
+        for key in self.store.keys():
+            rv[key] = self.get(key).value
         return rv
 
+
+class Config(dict):
+    def __init__(self):
+        self.config_vars = ConfigVars()
+
+    def __contains__(self, key):
+        return self.config_vars.exists(key)
+
+    def __setitem__(self, key, value):
+        self.config_vars.add(key, value=value)
+
+    def __getitem__(self, key):
+        var = self.config_vars.get(key)
+        if var.alias and var.alias != key:
+            raise KeyError(f"{key} is not a valid key.")
+        return var.value
+
+    def __str__(self):
+        return str(self.config_vars.to_dict())
+
     def from_object(self, obj: object):
         """Read configuration from an object."""
         _store = {key: value for key, value in obj.__dict__.items() if key.isupper()}
         for name, value in _store.items():
-            self[name] = value
+            self.config_vars.add(name, value=value)
 
     def from_dotenv(self, path: str):
         """Load configuration from specified .env path."""
-        _store = dotenv_to_dict(path)
-        for name, value in _store.items():
-            self[name] = value
+        for name, value in dotenv_to_dict(path).items():
+            self.config_vars.add(name, value=value)
 
     def from_envar(self, name: str, rename: str = None, default: str = None):
         """Set configuration from an environment variable."""
-        if not os.getenv(name):
+        if not os.getenv(name) and rename is None:
             warn_msg = f"Environment variable '{name}' not found."
             warnings.warn(warn_msg, RuntimeWarning)
             return
-        if rename is not None:
-            self._aliases[rename] = name
-        if default is not None:
-            self._defaults[name] = default
+        self.config_vars.add(name, alias=rename, default=default, use_env=True)
diff --git a/configureme/helpers.py b/configureme/helpers.py
index 2e724cd..09d711c 100644
--- a/configureme/helpers.py
+++ b/configureme/helpers.py
@@ -1,4 +1,5 @@
 # -*- coding: utf-8 -*-
+import os
 from typing import Any, Dict
 
 BOOLEAN_STATES = {"true": True, "false": False, "True": True, "False": False}
@@ -80,3 +81,17 @@ def dotenv_to_dict(path: str) -> Dict[str, Any]:
         py_value = str_to_py(value)
         rv[name] = py_value
     return rv
+
+
+def get_envar(name: str) -> Any:
+    rv = None
+    try:
+        env_value = os.environ[name]
+        rv = str_to_py(env_value)
+    except KeyError:
+        pass
+    return rv
+
+
+def envar_exists(name: str) -> bool:
+    return os.getenv(name) is not None


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 292a5bf..f09bf61 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -10,167 +10,125 @@ from .config import DevConfig
 pytestmark = pytest.mark.unit
 
 
-def setup_function():
-    try:
+class TestConfig:
+    def test_from_object(self):
+        config = Config()
+        config.from_object(DevConfig)
+        assert "DEBUG" in config
+        assert config["DEBUG"] is True
+
+    def test_from_dotenv(self, env_path):
+        config = Config()
+        config.from_dotenv(env_path)
+        assert "DEBUG" in config
+        assert config["DEBUG"] is True
+
+    def test_from_obj_and_then_dotenv(self, env_path):
+        config = Config()
+        config.from_dotenv(env_path)
+        config.from_object(DevConfig)
+        assert "DEBUG" in config
+        assert "TESTING" in config
+        assert config["DEBUG"] is True
+        assert config["TESTING"] is False
+
+    def test_from_envar_that_doesnt_exist_raises_warning(self):
+        config = Config()
+        with pytest.warns(RuntimeWarning) as warn:
+            config.from_envar("BAR")
+
+        assert len(warn) == 1
+        assert "not found" in warn[0].message.args[0]
+
+    def test_from_envar_that_doesnt_exist_but_exists_in_config_object(self):
+        config = Config()
+        config["BAR"] = "BAZ"
+        config.from_envar("BAR")
+        assert "BAR" in config
+        assert config["BAR"] == "BAZ"
+
+    def test_from_envar_and_change_after_setting_value(self):
+        os.environ["FOO"] = "BAR"
+        config = Config()
+        config.from_envar("FOO")
+        assert config["FOO"] == "BAR"
+        os.environ["FOO"] = "BAZ"
+        assert config["FOO"] == "BAZ"
         del os.environ["FOO"]
-    except KeyError:
-        pass
-    os.environ["FOO"] = "BAR"
-
 
-def teardown_function():
-    try:
+    def test_env_var_is_set_then_gets_removed_without_default_raises_exception(self):
+        os.environ["FOO"] = "BAR"
+        config = Config()
+        config.from_envar("FOO")
         del os.environ["FOO"]
-    except KeyError:
-        pass
-
-
-def test_from_object():
-    config = Config()
-    config.from_object(DevConfig)
-    assert "DEBUG" in config
-    assert config["DEBUG"] is True
-
-
-def test_from_dotenv(env_path):
-    config = Config()
-    config.from_dotenv(env_path)
-    assert "DEBUG" in config
-    assert config["DEBUG"] is True
-
-
-def test_from_obj_and_then_dotenv(env_path):
-    config = Config()
-    config.from_dotenv(env_path)
-    config.from_object(DevConfig)
-    assert "DEBUG" in config
-    assert "TESTING" in config
-    assert config["DEBUG"] is True
-    assert config["TESTING"] is False
-
-
-def test_from_envar_that_doesnt_exist_raises_warning():
-    config = Config()
-    with pytest.warns(RuntimeWarning) as warn:
-        config.from_envar("BAR")
-
-    assert len(warn) == 1
-    assert "not found" in warn[0].message.args[0]
-
-
-def test_from_envar_that_doesnt_exist_but_exists_in_config_object():
-    config = Config()
-    config["BAR"] = "BAZ"
-    config.from_envar("BAR")
-    assert "BAR" in config
-    assert config["BAR"] == "BAZ"
-
-
-def test_from_envar_and_change_after_setting_value():
-    config = Config()
-    config.from_envar("FOO")
-    assert config["FOO"] == "BAR"
-    os.environ["FOO"] = "BAZ"
-    assert config["FOO"] == "BAZ"
-
-
-def test_env_var_is_set_then_gets_removed_without_default_raises_exception():
-    config = Config()
-    config.from_envar("FOO")
-    del os.environ["FOO"]
-    with pytest.raises(ValueError):
-        config["FOO"]
-
-
-def test_env_var_is_set_then_gets_removed_with_config_value_set():
-    config = Config()
-    config.from_object(DevConfig)
-    config.from_envar("FOO")
-    assert config["FOO"] == "BAR"
-    del os.environ["FOO"]
-    assert config["FOO"] == "BAZ"
-
-
-def test_user_can_change_value_manually():
-    config = Config()
-    config["BAZ"] = "FOO"
-    assert config["BAZ"] == "FOO"
-
-
-def test_user_can_change_value_manually_but_env_value_takes_precedence():
-    config = Config()
-    config["FOO"] = "BAZ"
-    assert config["FOO"] == "BAR"
-
-
-def test_user_can_use_number_types_with_dotenv(env_path):
-    config = Config()
-    config.from_dotenv(env_path)
-    assert isinstance(config["MAX_LINES"], int)
-    assert config["MAX_LINES"] == 10
-    assert isinstance(config["TEMPERATURE"], float)
-    assert config["TEMPERATURE"] == 98.2
-
-
-def test_dunder_str():
-    expected = "{'DEBUG': True}"
-    config = Config()
-    config["DEBUG"] = True
-    assert str(config) == expected
-
-
-def test_dunder_eq():
-    config_one = Config()
-    config_one["DEBUG"] = True
-
-    config_two = Config()
-    config_two["DEBUG"] = True
-
-    assert config_one == config_two
-
-
-def test_dunder_ne():
-    config_one = Config()
-    config_one["DEBUG"] = True
-
-    config_two = Config()
-    config_two["DEBUG"] = False
-
-    assert config_one != config_two
-
-
-def test_env_var_can_be_renamed():
-    try:
-        del os.environ["FOO_DEBUG"]
-    except KeyError:
-        pass
-    assert os.getenv("FOO_DEBUG") is None
-    os.environ["FOO_DEBUG"] = "true"
-    config = Config()
-    config.from_envar("FOO_DEBUG", rename="DEBUG")
-    assert config["DEBUG"] is True
-
-
-def test_from_env_var_that_equals_not_found():
-    config = Config()
-    os.environ["FOO"] = "not found"
-    config.from_envar("FOO")
-    assert config["FOO"] == "not found"
-
-
-def test_from_env_var_with_default_value():
-    config = Config()
-    os.environ["FOO"] = "BAR"
-    config.from_envar("FOO", default="BAZ")
-    assert config["FOO"] == "BAR"
-    del os.environ["FOO"]
-    assert config["FOO"] == "BAZ"
-
+        with pytest.raises(KeyError):
+            config["FOO"]
+
+    def test_env_var_is_set_then_gets_removed_with_config_value_set(self):
+        os.environ["FOO"] = "BAR"
+        config = Config()
+        config.from_object(DevConfig)
+        config.from_envar("FOO")
+        assert config["FOO"] == "BAR"
+        del os.environ["FOO"]
+        assert config["FOO"] == "BAZ"
+
+    def test_user_can_change_value_manually(self):
+        config = Config()
+        config["BAZ"] = "FOO"
+        assert config["BAZ"] == "FOO"
+
+    def test_user_can_change_value_manually_but_env_value_takes_precedence(self):
+        os.environ["FOO"] = "expected"
+        config = Config()
+        config["FOO"] = "not expected"
+        config.from_envar("FOO")
+        assert config["FOO"] == "expected"
+
+    def test_user_can_use_number_types_with_dotenv(self, env_path):
+        config = Config()
+        config.from_dotenv(env_path)
+        assert isinstance(config["MAX_LINES"], int)
+        assert config["MAX_LINES"] == 10
+        assert isinstance(config["TEMPERATURE"], float)
+        assert config["TEMPERATURE"] == 98.2
+
+    def test_dunder_str(self):
+        expected = "{'DEBUG': True}"
+        config = Config()
+        config["DEBUG"] = True
+        assert str(config) == expected
+
+    def test_env_var_can_be_renamed(self):
+        try:
+            del os.environ["FOO_DEBUG"]
+        except KeyError:
+            pass
+        os.environ["FOO_DEBUG"] = "true"
+        config = Config()
+        config.from_envar("FOO_DEBUG", rename="DEBUG")
+        assert config["DEBUG"] is True
+
+    def test_from_env_var_that_equals_not_found(self):
+        config = Config()
+        os.environ["FOO"] = "not found"
+        config.from_envar("FOO")
+        assert config["FOO"] == "not found"
+
+    def test_from_env_var_with_default_value(self):
+        config = Config()
+        os.environ["FOO"] = "BAR"
+        config.from_envar("FOO", default="BAZ")
+        assert config["FOO"] == "BAR"
+        del os.environ["FOO"]
+        assert config["FOO"] == "BAZ"
 
-def test_from_env_var_with_default_value_and_alias():
-    config = Config()
-    os.environ["FOO"] = "BAR"
-    config.from_envar("FOO", rename="FOOBAR", default="BAZ")
-    assert config["FOOBAR"] == "BAR"
-    del os.environ["FOO"]
-    assert config["FOOBAR"] == "BAZ"
+    def test_from_env_var_with_default_value_and_alias(self):
+        config = Config()
+        os.environ["FOO"] = "BAR"
+        config.from_envar("FOO", rename="FOOBAR", default="BAZ")
+        assert config["FOOBAR"] == "BAR"
+        del os.environ["FOO"]
+        assert config["FOOBAR"] == "BAZ"
+        with pytest.raises(KeyError):
+            config["FOO"]  # should not be part of config
diff --git a/tests/test_helpers.py b/tests/test_helpers.py
index b484bb6..16b86a9 100644
--- a/tests/test_helpers.py
+++ b/tests/test_helpers.py
@@ -1,4 +1,6 @@
 # -*- coding: utf-8 -*-
+import os
+
 import pytest
 
 from configureme import helpers
@@ -87,3 +89,23 @@ def test_dotenv_to_dict(env_path):
     }
     actual = helpers.dotenv_to_dict(env_path)
     assert actual == expected
+
+
+def test_get_envar_that_doesnt_exist():
+    assert helpers.get_envar("BLAH") is None
+
+
+def test_get_envar_that_does_exist():
+    os.environ["FOO"] = "true"
+    assert helpers.get_envar("FOO") is True
+    del os.environ["FOO"]
+
+
+def test_envar_exists_is_false_when_no_var_exists():
+    assert helpers.envar_exists("BLAH") is False
+
+
+def test_envar_exists_is_true_when_env_var_exists():
+    os.environ["FOO"] = "true"
+    assert helpers.envar_exists("FOO") is True
+    del os.environ["FOO"]