Configuration strict mode raise on non existing files (#375)

* Update from_yaml()

* Refactor YAML environment variables interpolation

* Update from_ini()

* Refactor UNDEFINED

* Update from_env()

* Update from_dict()

* Update docs

* Update changelog

docs/main/changelog.rst

@@ -13,6 +13,8 @@ Development version
   default YAML loader.
 - Make security improvement: change default YAML loader to the custom ``yaml.SafeLoader`` with a support
   of environment variables interpolation.
+- Update configuration provider ``.from_*()`` methods to raise an exception in strict mode if
+  configuration file does not exist or configuration data is undefined.
 - Fix a bug with asynchronous injections: async providers do not work with async dependencies.
   See issue: `#368 <https://github.com/ets-labs/python-dependency-injector/issues/368>`_.
   Thanks `@kolypto <https://github.com/kolypto>`_ for the bug report.

docs/providers/configuration.rst

@@ -168,7 +168,42 @@ on access to any undefined option.
    :lines: 3-
    :emphasize-lines: 12
 
-You can also use ``.required()`` option modifier when making an injection.
+Methods ``.from_*()`` in strict mode raise an exception if configuration file does not exist or
+configuration data is undefined:
+
+.. code-block:: python
+   :emphasize-lines: 10,15,20,25
+
+   class Container(containers.DeclarativeContainer):
+
+       config = providers.Configuration(strict=True)
+
+
+   if __name__ == '__main__':
+       container = Container()
+
+       try:
+           container.config.from_yaml('./does-not_exist.yml')  # raise exception
+       except FileNotFoundError:
+           ...
+
+       try:
+           container.config.from_ini('./does-not_exist.ini')  # raise exception
+       except FileNotFoundError:
+           ...
+
+       try:
+           container.config.from_env('UNDEFINED_ENV_VAR')  # raise exception
+       except ValueError:
+           ...
+
+       try:
+           container.config.from_dict({})  # raise exception
+       except ValueError:
+           ...
+
+You can also use ``.required()`` option modifier when making an injection. It does not require to switch
+configuration provider to strict mode.
 
 .. literalinclude:: ../../examples/providers/configuration/configuration_required.py
    :language: python

src/dependency_injector/providers.pyx

@@ -3,6 +3,7 @@
 from __future__ import absolute_import
 
 import copy
+import errno
 import functools
 import inspect
 import os
@@ -53,14 +54,6 @@ else:  # pragma: no cover
                                     copy.deepcopy(obj.im_self, memo),
                                     obj.im_class)
 
-if yaml:
-    yaml_env_marker_pattern = re.compile(r'\$\{([^}^{]+)\}')
-    def yaml_env_marker_constructor(_, node):
-        """"Replace environment variable marker with its value."""
-        return os.path.expandvars(node.value)
-
-    yaml.add_implicit_resolver('!path', yaml_env_marker_pattern)
-    yaml.add_constructor('!path', yaml_env_marker_constructor)
 
 if sys.version_info[0] == 3:
     class EnvInterpolation(iniconfigparser.BasicInterpolation):
@@ -72,49 +65,38 @@ if sys.version_info[0] == 3:
 
     def _parse_ini_file(filepath):
         parser = iniconfigparser.ConfigParser(interpolation=EnvInterpolation())
-        parser.read(filepath)
+        with open(filepath) as config_file:
+            parser.read_file(config_file)
         return parser
 else:
     import StringIO
 
     def _parse_ini_file(filepath):
         parser = iniconfigparser.ConfigParser()
-        try:
-            with open(filepath) as config_file:
-                config_string = os.path.expandvars(config_file.read())
-        except IOError:
-            return parser
-        else:
-            parser.readfp(StringIO.StringIO(config_string))
-            return parser
+        with open(filepath) as config_file:
+            config_string = os.path.expandvars(config_file.read())
+        parser.readfp(StringIO.StringIO(config_string))
+        return parser
 
 
 if yaml:
+    # TODO: use SafeLoader without env interpolation by default in version 5.*
+    yaml_env_marker_pattern = re.compile(r'\$\{([^}^{]+)\}')
+    def yaml_env_marker_constructor(_, node):
+        """"Replace environment variable marker with its value."""
+        return os.path.expandvars(node.value)
+
+    yaml.add_implicit_resolver('!path', yaml_env_marker_pattern)
+    yaml.add_constructor('!path', yaml_env_marker_constructor)
+
     class YamlLoader(yaml.SafeLoader):
         """Custom YAML loader.
 
         Inherits ``yaml.SafeLoader`` and add environment variables interpolation.
         """
 
-        tag = '!!str'
-        pattern = re.compile('.*?\${(\w+)}.*?')
-
-        @classmethod
-        def constructor_env_variables(cls, loader, node):
-            value = loader.construct_scalar(node)
-            match = cls.pattern.findall(value)
-            if match:
-                full_value = value
-                for group in match:
-                    full_value = full_value.replace(
-                        f'${{{group}}}', os.environ.get(group, group)
-                    )
-                return full_value
-            return value
-
-    # TODO: use SafeLoader without env interpolation by default in version 5.*
-    YamlLoader.add_implicit_resolver(YamlLoader.tag, YamlLoader.pattern, None)
-    YamlLoader.add_constructor(YamlLoader.tag, YamlLoader.constructor_env_variables)
+    YamlLoader.add_implicit_resolver('!path', yaml_env_marker_pattern, None)
+    YamlLoader.add_constructor('!path', yaml_env_marker_constructor)
 else:
     class YamlLoader:
         """Custom YAML loader.
@@ -123,6 +105,7 @@ else:
         """
 
 
+UNDEFINED = object()
 
 cdef int ASYNC_MODE_UNDEFINED = 0
 cdef int ASYNC_MODE_ENABLED = 1
@@ -1205,14 +1188,12 @@ cdef class ConfigurationOption(Provider):
     :py:class:`Configuration` provider.
     """
 
-    UNDEFINED = object()
-
     def __init__(self, name, root, required=False):
         self.__name = name
         self.__root_ref = weakref.ref(root)
         self.__children = {}
         self.__required = required
-        self.__cache = self.UNDEFINED
+        self.__cache = UNDEFINED
         super().__init__()
 
     def __deepcopy__(self, memo):
@@ -1261,7 +1242,7 @@ cdef class ConfigurationOption(Provider):
 
     cpdef object _provide(self, tuple args, dict kwargs):
         """Return new instance."""
-        if self.__cache is not self.UNDEFINED:
+        if self.__cache is not UNDEFINED:
             return self.__cache
 
         root = self.__root_ref()
@@ -1313,7 +1294,7 @@ cdef class ConfigurationOption(Provider):
         raise Error('Configuration option does not support this method')
 
     def reset_cache(self):
-        self.__cache = self.UNDEFINED
+        self.__cache = UNDEFINED
         for child in self.__children.values():
             child.reset_cache()
 
@@ -1341,7 +1322,13 @@ cdef class ConfigurationOption(Provider):
 
         :rtype: None
         """
-        parser = _parse_ini_file(filepath)
+        try:
+            parser = _parse_ini_file(filepath)
+        except IOError as exception:
+            if self._is_strict_mode_enabled() and exception.errno in (errno.ENOENT, errno.EISDIR):
+                exception.strerror = 'Unable to load configuration file {0}'.format(exception.strerror)
+                raise
+            return
 
         config = {}
         for section in parser.sections():
@@ -1379,7 +1366,10 @@ cdef class ConfigurationOption(Provider):
         try:
             with open(filepath) as opened_file:
                 config = yaml.load(opened_file, loader)
-        except IOError:
+        except IOError as exception:
+            if self._is_strict_mode_enabled() and exception.errno in (errno.ENOENT, errno.EISDIR):
+                exception.strerror = 'Unable to load configuration file {0}'.format(exception.strerror)
+                raise
             return
 
         current_config = self.__call__()
@@ -1397,25 +1387,43 @@ cdef class ConfigurationOption(Provider):
 
         :rtype: None
         """
+        if self._is_strict_mode_enabled() and not options:
+            raise ValueError('Can not use empty dictionary')
+
         current_config = self.__call__()
         if not current_config:
             current_config = {}
         self.override(merge_dicts(current_config, options))
 
-    def from_env(self, name, default=None):
+    def from_env(self, name, default=UNDEFINED):
         """Load configuration value from the environment variable.
 
         :param name: Name of the environment variable.
         :type name: str
 
         :param default: Default value that is used if environment variable does not exist.
-        :type default: str
+        :type default: object
 
         :rtype: None
         """
-        value = os.getenv(name, default)
+        value = os.environ.get(name, default)
+
+        if value is UNDEFINED:
+            if self._is_strict_mode_enabled():
+                raise ValueError('Environment variable "{0}" is undefined'.format(name))
+            value = None
+
         self.override(value)
 
+    def _is_strict_mode_enabled(self):
+        cdef Configuration root
+
+        root = self.__root_ref()
+        if not root:
+            return False
+
+        return root.__strict
+
 
 cdef class TypedConfigurationOption(Callable):
 
@@ -1445,7 +1453,6 @@ cdef class Configuration(Object):
     """
 
     DEFAULT_NAME = 'config'
-    UNDEFINED = object()
 
     def __init__(self, name=DEFAULT_NAME, default=None, strict=False):
         self.__name = name
@@ -1516,17 +1523,17 @@ cdef class Configuration(Object):
         value = self.__call__()
 
         if value is None:
-            if self.__strict or required:
+            if self._is_strict_mode_enabled() or required:
                 raise Error('Undefined configuration option "{0}.{1}"'.format(self.__name, selector))
             return None
 
         keys = selector.split('.')
         while len(keys) > 0:
             key = keys.pop(0)
-            value = value.get(key, self.UNDEFINED)
+            value = value.get(key, UNDEFINED)
 
-            if value is self.UNDEFINED:
-                if self.__strict or required:
+            if value is UNDEFINED:
+                if self._is_strict_mode_enabled() or required:
                     raise Error('Undefined configuration option "{0}.{1}"'.format(self.__name, selector))
                 return None
 
@@ -1624,7 +1631,13 @@ cdef class Configuration(Object):
 
         :rtype: None
         """
-        parser = _parse_ini_file(filepath)
+        try:
+            parser = _parse_ini_file(filepath)
+        except IOError as exception:
+            if self._is_strict_mode_enabled() and exception.errno in (errno.ENOENT, errno.EISDIR):
+                exception.strerror = 'Unable to load configuration file {0}'.format(exception.strerror)
+                raise
+            return
 
         config = {}
         for section in parser.sections():
@@ -1661,7 +1674,10 @@ cdef class Configuration(Object):
         try:
             with open(filepath) as opened_file:
                 config = yaml.load(opened_file, loader)
-        except IOError:
+        except IOError as exception:
+            if self._is_strict_mode_enabled() and exception.errno in (errno.ENOENT, errno.EISDIR):
+                exception.strerror = 'Unable to load configuration file {0}'.format(exception.strerror)
+                raise
             return
 
         current_config = self.__call__()
@@ -1679,25 +1695,37 @@ cdef class Configuration(Object):
 
         :rtype: None
         """
+        if self._is_strict_mode_enabled() and not options:
+            raise ValueError('Can not use empty dictionary')
+
         current_config = self.__call__()
         if not current_config:
             current_config = {}
         self.override(merge_dicts(current_config, options))
 
-    def from_env(self, name, default=None):
+    def from_env(self, name, default=UNDEFINED):
         """Load configuration value from the environment variable.
 
         :param name: Name of the environment variable.
         :type name: str
 
         :param default: Default value that is used if environment variable does not exist.
-        :type default: str
+        :type default: object
 
         :rtype: None
         """
-        value = os.getenv(name, default)
+        value = os.environ.get(name, default)
+
+        if value is UNDEFINED:
+            if self._is_strict_mode_enabled():
+                raise ValueError('Environment variable "{0}" is undefined'.format(name))
+            value = None
+
         self.override(value)
 
+    def _is_strict_mode_enabled(self):
+        return self.__strict
+
 
 cdef class Factory(Provider):
     r"""Factory provider creates new instance on every call.

tests/unit/providers/test_configuration_py2_py3.py

@@ -426,6 +426,24 @@ class ConfigFromIniTests(unittest.TestCase):
         self.assertEqual(self.config.section3(), {'value3': '3'})
         self.assertEqual(self.config.section3.value3(), '3')
 
+    def test_file_does_not_exist(self):
+        self.config.from_ini('./does_not_exist.ini')
+        self.assertEqual(self.config(), {})
+
+    def test_file_does_not_exist_strict_mode(self):
+        self.config = providers.Configuration(strict=True)
+        with self.assertRaises(IOError):
+            self.config.from_ini('./does_not_exist.ini')
+
+    def test_option_file_does_not_exist(self):
+        self.config.option.from_ini('does_not_exist.ini')
+        self.assertIsNone(self.config.option.undefined())
+
+    def test_option_file_does_not_exist_strict_mode(self):
+        self.config = providers.Configuration(strict=True)
+        with self.assertRaises(IOError):
+            self.config.option.from_ini('./does_not_exist.ini')
+
 
 class ConfigFromIniWithEnvInterpolationTests(unittest.TestCase):
 
@@ -529,6 +547,24 @@ class ConfigFromYamlTests(unittest.TestCase):
         self.assertEqual(self.config.section3(), {'value3': 3})
         self.assertEqual(self.config.section3.value3(), 3)
 
+    def test_file_does_not_exist(self):
+        self.config.from_yaml('./does_not_exist.yml')
+        self.assertEqual(self.config(), {})
+
+    def test_file_does_not_exist_strict_mode(self):
+        self.config = providers.Configuration(strict=True)
+        with self.assertRaises(IOError):
+            self.config.from_yaml('./does_not_exist.yml')
+
+    def test_option_file_does_not_exist(self):
+        self.config.option.from_yaml('./does_not_exist.yml')
+        self.assertIsNone(self.config.option())
+
+    def test_option_file_does_not_exist_strict_mode(self):
+        self.config = providers.Configuration(strict=True)
+        with self.assertRaises(IOError):
+            self.config.option.from_yaml('./does_not_exist.yml')
+
     def test_no_yaml_installed(self):
         @contextlib.contextmanager
         def no_yaml_module():
@@ -663,6 +699,24 @@ class ConfigFromDict(unittest.TestCase):
         self.assertEqual(self.config.section2(), {'value2': '2'})
         self.assertEqual(self.config.section2.value2(), '2')
 
+    def test_empty_dict(self):
+        self.config.from_dict({})
+        self.assertEqual(self.config(), {})
+
+    def test_option_empty_dict(self):
+        self.config.option.from_dict({})
+        self.assertEqual(self.config.option(), {})
+
+    def test_empty_dict_in_strict_mode(self):
+        self.config = providers.Configuration(strict=True)
+        with self.assertRaises(ValueError):
+            self.config.from_dict({})
+
+    def test_option_empty_dict_in_strict_mode(self):
+        self.config = providers.Configuration(strict=True)
+        with self.assertRaises(ValueError):
+            self.config.option.from_dict({})
+
     def test_merge(self):
         self.config.from_dict(self.config_options_1)
         self.config.from_dict(self.config_options_2)
@@ -709,6 +763,34 @@ class ConfigFromEnvTests(unittest.TestCase):
         self.config.from_env('UNDEFINED_ENV', 'default-value')
         self.assertEqual(self.config(), 'default-value')
 
+    def test_undefined_in_strict_mode(self):
+        self.config = providers.Configuration(strict=True)
+        with self.assertRaises(ValueError):
+            self.config.from_env('UNDEFINED_ENV')
+
+    def test_option_undefined_in_strict_mode(self):
+        self.config = providers.Configuration(strict=True)
+        with self.assertRaises(ValueError):
+            self.config.option.from_env('UNDEFINED_ENV')
+
+    def test_undefined_in_strict_mode_with_default(self):
+        self.config = providers.Configuration(strict=True)
+        self.config.from_env('UNDEFINED_ENV', 'default-value')
+        self.assertEqual(self.config(), 'default-value')
+
+    def test_option_undefined_in_strict_mode_with_default(self):
+        self.config = providers.Configuration(strict=True)
+        self.config.option.from_env('UNDEFINED_ENV', 'default-value')
+        self.assertEqual(self.config.option(), 'default-value')
+
+    def test_default_none(self):
+        self.config.from_env('UNDEFINED_ENV')
+        self.assertIsNone(self.config())
+
+    def test_option_default_none(self):
+        self.config.option.from_env('UNDEFINED_ENV')
+        self.assertIsNone(self.config.option())
+
     def test_with_children(self):
         self.config.section1.value1.from_env('CONFIG_TEST_ENV')