Repository: rochacbruno/dynaconf
Pull request #249 (2019-10-23 08:16:08)

Fix env_loader.write's quote mode for non-string values
Dynaconf 2.2.0
Python 3.6.8

Assume the configuration from example in Section https://dynaconf.readthedocs.io/en/latest/guides/environment_variables.html#precedence-and-type-casting in toml-format
```
[default]
# Numbers
DYNACONF_INTEGER=42
DYNACONF_FLOAT=3.14

# Text
#DYNACONF_STRING=Hello
DYNACONF_STRING2="Hello"

# Booleans
DYNACONF_BOOL=true
DYNACONF_BOOL2=false

# Use extra quotes to force a string from other type
DYNACONF_STRING21="'42'"
DYNACONF_STRING22="'true'"

# Arrays must be homogenous in toml syntax
DYNACONF_ARRAY=[1, 2, 3]
DYNACONF_ARRAY2=[1.1, 2.2, 3.3]
DYNACONF_ARRAY3=['a', 'b', 'c']

# Dictionaries
DYNACONF_DICT={key="abc",val=123}```
```
After setting ` ENV_FOR_DYNACONF` to `default`, I list my settings with
```
dynaconf list  -o out.yaml --output-flat
```
Then `out.yaml ` contains:
```
DYNACONF_ARRAY:
- 1
- 2
- 3
DYNACONF_ARRAY2:
- 1.1
- 2.2
- 3.3
DYNACONF_ARRAY3:
- a
- b
- c
DYNACONF_BOOL: true
DYNACONF_BOOL2: false
DYNACONF_DICT:
  key: abc
  val: 123
DYNACONF_FLOAT: 3.14
DYNACONF_INTEGER: 42
DYNACONF_STRING2: Hello
DYNACONF_STRING21: '''42'''
DYNACONF_STRING22: '''true''' 
```
Those values are expected.

Now export it using the env-loader
```
dynaconf list  -o .env --output-flat 
```
the `.env` file holds
```
DYNACONF_INTEGER="42"
DYNACONF_FLOAT="3.14"
DYNACONF_STRING2="Hello"
DYNACONF_BOOL="True"
DYNACONF_BOOL2="False"
DYNACONF_STRING21="42"
DYNACONF_STRING22="true"
DYNACONF_ARRAY="[1, 2, 3]"
DYNACONF_ARRAY2="[1.1, 2.2, 3.3]"
DYNACONF_ARRAY3="['a', 'b', 'c']"
DYNACONF_DICT="{'key': 'abc', 'val': 123}" 
```
This unexpected and it also differs from the yaml-loader’s behavior.
Furthermore, this is surprising as the program output contains the following
```
Working in default environment 
DYNACONF_INTEGER: 42
DYNACONF_FLOAT: 3.14
DYNACONF_STRING2: 'Hello'
DYNACONF_BOOL: True
DYNACONF_BOOL2: False
DYNACONF_STRING21: "'42'"
DYNACONF_STRING22: "'true'"
DYNACONF_ARRAY: [1, 2, 3]
DYNACONF_ARRAY2: [1.1, 2.2, 3.3]
DYNACONF_ARRAY3: ['a', 'b', 'c']
DYNACONF_DICT: {'key': 'abc', 'val': 123} 
```

The culprit seems to be `write` in `env_loader`, https://github.com/rochacbruno/dynaconf/blob/master/dynaconf/loaders/env_loader.py#L63, it stringifies everything and then dotenv is quoting everything as this is `set_key`’s default behavior: `set_key(dotenv_path, key_to_set, value_to_set, quote_mode="always")`. 

The suggested fix could be simply setting the quote mode to only always quote strings: `quote_mode="always" if isinstance(value, string_types) else "not_always"`.

__Edit__: Actually, simple strings should not be quoted either. Maybe one should only quote those variables that "use extra quotes to force a string from other type" as well as lists and dicts and thus set `quote_mode` to `always` only if ` ` holds true.
That would change one line in the `.env` example above to: 
```
DYNACONF_STRING2=Hello
```
After commit `4d7849a`, the `.env` looks like:
```
DYNACONF_TESTING=true
DYNACONF_INTEGER=42
DYNACONF_FLOAT=3.14
DYNACONF_STRING2=Hello
DYNACONF_STRING2_LONG="Hello World!"
DYNACONF_BOOL=True
DYNACONF_BOOL2=False
DYNACONF_STRING21="42"
DYNACONF_STRING22="true"
DYNACONF_ARRAY="[1, 2, 3]"
DYNACONF_ARRAY2="[1.1, 2.2, 3.3]"
DYNACONF_ARRAY3="['a', 'b', 'c']"
DYNACONF_DICT="{'key': 'abc', 'val': 123}"
```


codecov[bot]: # [Codecov](https://codecov.io/gh/rochacbruno/dynaconf/pull/249?src=pr&el=h1) Report
> Merging [#249](https://codecov.io/gh/rochacbruno/dynaconf/pull/249?src=pr&el=desc) into [master](https://codecov.io/gh/rochacbruno/dynaconf/commit/de90d677cbd33c36884046c1dd63c86a3a5bc839?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/rochacbruno/dynaconf/pull/249/graphs/tree.svg?width=650&token=GXszVzJcqW&height=150&src=pr)](https://codecov.io/gh/rochacbruno/dynaconf/pull/249?src=pr&el=tree)

```diff
@@          Coverage Diff          @@
##           master   #249   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          18     18           
  Lines         784    785    +1     
=====================================
+ Hits          784    785    +1
```


| [Impacted Files](https://codecov.io/gh/rochacbruno/dynaconf/pull/249?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [dynaconf/loaders/env\_loader.py](https://codecov.io/gh/rochacbruno/dynaconf/pull/249/diff?src=pr&el=tree#diff-ZHluYWNvbmYvbG9hZGVycy9lbnZfbG9hZGVyLnB5) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/rochacbruno/dynaconf/pull/249?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/rochacbruno/dynaconf/pull/249?src=pr&el=footer). Last update [de90d67...c94e8e8](https://codecov.io/gh/rochacbruno/dynaconf/pull/249?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

rochacbruno: Python 3.5 is having a failure, we need to investigate the reason.

```bash
=================================== FAILURES ===================================
__________________________________ test_write __________________________________

tmpdir = local('/tmp/pytest-of-vsts/pytest-0/test_write0')

    def test_write(tmpdir):
        # settings
    
        settings_path = tmpdir.join(".env")
    
        write(settings_path, SETTINGS_DATA)
    
        ground_truth = SETTINGS_DATA_GROUND_TRUTH.split("\n")
    
>       with open(settings_path, "r") as fp:
E       TypeError: invalid file: local('/tmp/pytest-of-vsts/pytest-0/test_write0/.env')

ground_truth = ['DYNACONF_TESTING=true', 'DYNACONF_INTEGER=42', 'DYNACONF_FLOAT=3.14', 'DYNACONF_STRING2=Hello', 'DYNACONF_STRING2_LONG="Hello World!"', 'DYNACONF_BOOL=True', ...]
settings_path = local('/tmp/pytest-of-vsts/pytest-0/test_write0/.env')
tmpdir     = local('/tmp/pytest-of-vsts/pytest-0/test_write0')

/home/vsts/work/1/s/tests/test_env_loader.py:76: TypeError
-------- generated xml file: /home/vsts/work/1/s/junit/test-results.xml --------
```

Patch:
diff --git a/.gitignore b/.gitignore
index 0d281b5..b2efe43 100644
--- a/.gitignore
+++ b/.gitignore
@@ -37,6 +37,7 @@ var/
 pip-log.txt
 pip-delete-this-directory.txt
 Pipfile.lock
+Pipfile
 
 # Unit test / coverage reports
 htmlcov/
diff --git a/docs/index.rst b/docs/index.rst
index a23b58b..e628c73 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -96,6 +96,7 @@ Who is using Dynaconf?
 - Quokka CMS - Flask
 - iNNOVO Cloud GmbH
 - Teraki
+- ARA Records Ansible - Django
 
 Are you using Dynaconf? Please `give feedback`_
 
diff --git a/dynaconf/base.py b/dynaconf/base.py
index 821207f..b9ef6e4 100644
--- a/dynaconf/base.py
+++ b/dynaconf/base.py
@@ -256,26 +256,34 @@ class Settings(object):
 
     to_dict = as_dict  # backwards compatibility
 
-    def _dotted_get(self, dotted_key, default=None, **kwargs):
+    def _dotted_get(self, dotted_key, default=None, parent=None, **kwargs):
         """
         Perform dotted key lookups and keep track of where we are.
+        :param key: The name of the setting value, will always be upper case
+        :param default: In case of not found it will be returned
+        :param parent: Is there a pre-loaded parent in a nested data?
         """
         split_key = dotted_key.split(".")
         name, keys = split_key[0], split_key[1:]
-        result = self.get(name, default=default, **kwargs)
-        self._memoized = result
+        result = self.get(name, default=default, parent=parent, **kwargs)
 
-        # If we've reached the end, then return result and clear the
-        # memoized data.
-        if not keys or result is default:
-            self._memoized = None
+        # If we've reached the end, or parent key not found, then return result
+        if not keys or result == default:
             return result
 
         # If we've still got key elements to traverse, let's do that.
-        return self._dotted_get(".".join(keys), default=default, **kwargs)
+        return self._dotted_get(
+            ".".join(keys), default=default, parent=result, **kwargs
+        )
 
     def get(
-        self, key, default=None, cast=None, fresh=False, dotted_lookup=True
+        self,
+        key,
+        default=None,
+        cast=None,
+        fresh=False,
+        dotted_lookup=True,
+        parent=None,
     ):
         """
         Get a value from settings store, this is the prefered way to access::
@@ -288,12 +296,17 @@ class Settings(object):
         :param cast: Should cast in to @int, @float, @bool or @json ?
         :param fresh: Should reload from loaders store before access?
         :param dotted_lookup: Should perform dotted-path lookup?
+        :param parent: Is there a pre-loaded parent in a nested data?
         :return: The value if found, default or None
         """
 
         if "." in key and dotted_lookup:
             return self._dotted_get(
-                dotted_key=key, default=default, cast=cast, fresh=fresh
+                dotted_key=key,
+                default=default,
+                cast=cast,
+                fresh=fresh,
+                parent=parent,
             )
 
         key = upperfy(key)
@@ -308,8 +321,7 @@ class Settings(object):
             self.unset(key)
             self.execute_loaders(key=key)
 
-        store = self._memoized or self.store
-        data = store.get(key, default)
+        data = (parent or self.store).get(key, default)
         if cast:
             data = converters.get(cast)(data)
         return data
diff --git a/dynaconf/loaders/env_loader.py b/dynaconf/loaders/env_loader.py
index 772a541..7f91ab3 100644
--- a/dynaconf/loaders/env_loader.py
+++ b/dynaconf/loaders/env_loader.py
@@ -60,4 +60,13 @@ def load_from_env(identifier, key, env, obj, silent):
 def write(settings_path, settings_data, **kwargs):
     """Write data to .env file"""
     for key, value in settings_data.items():
-        dotenv_cli.set_key(str(settings_path), key, str(value))
+        quote_mode = (
+            isinstance(value, str)
+            and (value.startswith("'") or value.startswith('"'))
+        ) or isinstance(value, (list, dict))
+        dotenv_cli.set_key(
+            str(settings_path),
+            key,
+            str(value),
+            quote_mode="always" if quote_mode else "none",
+        )
diff --git a/dynaconf/validator.py b/dynaconf/validator.py
index d0e4ac6..abc44ca 100644
--- a/dynaconf/validator.py
+++ b/dynaconf/validator.py
@@ -83,6 +83,26 @@ class Validator(object):
         else:
             self.envs = None
 
+    def __eq__(self, other):
+        if self is other:
+            return True
+
+        identical_attrs = (
+            getattr(self, attr) == getattr(other, attr)
+            for attr in (
+                "names",
+                "must_exist",
+                "when",
+                "condition",
+                "operations",
+                "envs",
+            )
+        )
+        if all(identical_attrs):
+            return True
+
+        return False
+
     def validate(self, settings):
         """Raise ValidationError if invalid"""
 
@@ -160,7 +180,9 @@ class ValidatorList(list):
         self.settings = settings
 
     def register(self, *args):
-        self.extend(args)
+        for validator in args:
+            if validator not in self:
+                self.append(validator)
 
     def validate(self):
         for validator in self:
diff --git a/example/issues/251_dotted_unexistent/.env b/example/issues/251_dotted_unexistent/.env
new file mode 100644
index 0000000..e69de29
diff --git a/example/issues/251_dotted_unexistent/app.py b/example/issues/251_dotted_unexistent/app.py
new file mode 100644
index 0000000..54dec44
--- /dev/null
+++ b/example/issues/251_dotted_unexistent/app.py
@@ -0,0 +1,26 @@
+from dynaconf import settings
+
+print("As dict:")
+print(settings.as_dict())
+
+assert settings.FOO == {"bar": "hello"}
+assert settings.FOO.bar == "hello"
+
+assert settings.get("FOO") == {"bar": "hello"}
+assert settings.get("foo") == {"bar": "hello"}
+assert settings.get("foo.bar") == "hello", settings.get("foo.bar")
+
+assert settings.get("foo.zaz") is None
+assert settings.get("non.zaz") is None
+assert settings.get("non.zaz", 3) == 3
+print("With list as default 1:")
+assert settings.get("non.existing.key", [1, 2, 3]) == [1, 2, 3]
+assert settings.get("non.zaz", 3) == 3
+
+assert settings.get("foo.bar") == "hello"
+
+print("With 0 as default:")
+assert settings.get("non.existing.key", 0) == 0
+
+print("With list as default 2:")
+assert settings.get("non.existing.key", [4, 5, 6]) == [4, 5, 6]
diff --git a/example/issues/251_dotted_unexistent/settings.py b/example/issues/251_dotted_unexistent/settings.py
new file mode 100644
index 0000000..80f7f32
--- /dev/null
+++ b/example/issues/251_dotted_unexistent/settings.py
@@ -0,0 +1,1 @@
+FOO = {"bar": "hello"}


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index e09164c..1802a9d 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -49,7 +49,7 @@ def go_to_tmpdir(request):
 
     # Get the fixture dynamically by its name.
     tmpdir = request.getfixturevalue("tmpdir")
-    tmpdir.join(".env").write("DYNACONF_TESTING=true")
+    tmpdir.join(".env").write("DYNACONF_TESTING=true\n")
     # ensure local test created packages can be imported
     sys.path.insert(0, str(tmpdir))
 
diff --git a/tests/test_env_loader.py b/tests/test_env_loader.py
index 2775c98..e8c3981 100644
--- a/tests/test_env_loader.py
+++ b/tests/test_env_loader.py
@@ -1,5 +1,6 @@
 import os
 import sys
+from collections import OrderedDict
 from os import environ
 
 import pytest
@@ -7,6 +8,7 @@ import pytest
 from dynaconf import settings  # noqa
 from dynaconf.loaders.env_loader import load
 from dynaconf.loaders.env_loader import load_from_env
+from dynaconf.loaders.env_loader import write
 
 # GLOBAL ENV VARS
 environ["DYNACONF_HOSTNAME"] = "host.com"
@@ -26,6 +28,57 @@ settings.configure(
     ROOT_PATH_FOR_DYNACONF=os.path.dirname(os.path.abspath(__file__)),
 )
 
+SETTINGS_DATA = OrderedDict()
+# Numbers
+SETTINGS_DATA["DYNACONF_INTEGER"] = 42
+SETTINGS_DATA["DYNACONF_FLOAT"] = 3.14
+# Text
+# 'DYNACONF_STRING': Hello,
+SETTINGS_DATA["DYNACONF_STRING2"] = "Hello"
+SETTINGS_DATA["DYNACONF_STRING2_LONG"] = "Hello World!"
+SETTINGS_DATA["DYNACONF_BOOL"] = True
+SETTINGS_DATA["DYNACONF_BOOL2"] = False
+# Use extra quotes to force a string from other type
+SETTINGS_DATA["DYNACONF_STRING21"] = '"42"'
+SETTINGS_DATA["DYNACONF_STRING22"] = "'true'"
+# Arrays must be homogeneous in toml syntax
+SETTINGS_DATA["DYNACONF_ARRAY"] = [1, 2, 3]
+SETTINGS_DATA["DYNACONF_ARRAY2"] = [1.1, 2.2, 3.3]
+SETTINGS_DATA["DYNACONF_ARRAY3"] = ["a", "b", "c"]
+# Dictionaries
+SETTINGS_DATA["DYNACONF_DICT"] = {"val": 123}
+
+SETTINGS_DATA_GROUND_TRUTH = """DYNACONF_TESTING=true
+DYNACONF_INTEGER=42
+DYNACONF_FLOAT=3.14
+DYNACONF_STRING2=Hello
+DYNACONF_STRING2_LONG="Hello World!"
+DYNACONF_BOOL=True
+DYNACONF_BOOL2=False
+DYNACONF_STRING21="42"
+DYNACONF_STRING22="true"
+DYNACONF_ARRAY="[1, 2, 3]"
+DYNACONF_ARRAY2="[1.1, 2.2, 3.3]"
+DYNACONF_ARRAY3="[\'a\', \'b\', \'c\']"
+DYNACONF_DICT="{\'val\': 123}"
+"""
+
+
+def test_write(tmpdir):
+    settings_path = tmpdir.join(".env")
+
+    write(settings_path, SETTINGS_DATA)
+
+    ground_truth = SETTINGS_DATA_GROUND_TRUTH.split("\n")
+
+    with open(str(settings_path), "r") as fp:
+        lines = fp.readlines()
+        for idx, line in enumerate(lines):
+            line = line.strip()
+            if line.split("=")[0] == "DYNACONF_TESTING":
+                continue  # this key is written by the test itself; skip.
+            assert line == ground_truth[idx].strip()
+
 
 def test_env_loader():
     assert settings.HOSTNAME == "host.com"
diff --git a/tests/test_validators.py b/tests/test_validators.py
index aea45eb..c1d6252 100644
--- a/tests/test_validators.py
+++ b/tests/test_validators.py
@@ -177,3 +177,42 @@ def test_no_reload_on_single_env(tmpdir, mocker):
     settings.validators.validate()
     using_env.assert_any_call("production")
     assert using_env.call_count == 1
+
+
+def test_equality():
+    validator1 = Validator("VERSION", "AGE", "NAME", must_exist=True)
+    validator2 = Validator("VERSION", "AGE", "NAME", must_exist=True)
+
+    assert validator1 == validator2
+    assert validator1 is not validator2
+
+    validator3 = (
+        Validator("IMAGE_1", when=Validator("BASE_IMAGE", must_exist=True),),
+    )
+    validator4 = (
+        Validator("IMAGE_1", when=Validator("MYSQL_HOST", must_exist=True),),
+    )
+
+    assert validator3 != validator4
+
+
+def test_ignoring_duplicate_validators(tmpdir):
+    tmpfile = tmpdir.join("settings.toml")
+    tmpfile.write(TOML)
+
+    settings = LazySettings(
+        ENV_FOR_DYNACONF="EXAMPLE",
+        SETTINGS_FILE_FOR_DYNACONF=str(tmpfile),
+        silent=True,
+    )
+
+    validator1 = Validator("VERSION", "AGE", "NAME", must_exist=True)
+    settings.validators.register(
+        validator1, Validator("VERSION", "AGE", "NAME", must_exist=True),
+    )
+
+    assert len(settings.validators) == 1
+
+    settings.validators.register(validator1)
+
+    assert len(settings.validators) == 1