Refactored the way the Environment class stores configuration. The

set_config/get_config methods are gone, instead there is a "config"
property which exposes a dict-like object.

The Environment subclass used by django-assets can now more cleanly
implement a custom config backend, no longer needing to hack
__getattribute__.

Author: miracle2k

docs/environment.rst

@@ -72,12 +72,12 @@ Filter configuration
 ====================
 
 In addition to the standard options listed above, you can set custom
-configuration values using ``set_config``. This is so that you can
+configuration values using ``Environment.config``. This is so that you can
 configure filters through the environment:
 
 .. code-block:: python
 
-    environment.set_config('sass_bin', '/opt/sass/bin/sass')
+    environment.config['sass_bin'] = '/opt/sass/bin/sass')
 
 This allows the :ref:`Sass filter <filters-sass>` to find the sass
 binary.

src/django_assets/env.py

@@ -1,20 +1,14 @@
 import imp
 from django.conf import settings
-from webassets.env import Environment
+from webassets.env import Environment, ConfigStorage
 from webassets.importlib import import_module
 
 
 __all__ = ('register',)
 
 
-class DjangoEnvironment(Environment):
-    """For Django, we need to redirect all the configuration values this
-    object holds to Django's own settings object.
 
-    We do this by hooking into __getattribute__ and __setattribute__,
-    rather than reimplementing the get_foo/set_foo() methods, which means
-    we won't have to reimplement any validation the parent class may do.
-    """
+class DjangoConfigStorage(ConfigStorage):
 
     _mapping = {
         'debug': 'ASSETS_DEBUG',
@@ -26,37 +20,36 @@ class DjangoEnvironment(Environment):
         'url': 'MEDIA_URL',
     }
 
-    def __init__(self):
-        # Have the parent initialize the default values
-        super(DjangoEnvironment, self).__init__(None, None)
+    def _transform_key(self, key):
+        return self._mapping.get(key.lower(), key.upper())
 
-        # Then, from this point on, redirect various attributes
-        # to the Django settings object.
-        self.django_settings = settings
+    def __getitem__(self, key):
+        return getattr(settings, self._transform_key(key))
 
-    def get_config(self, key, default=None):
-        return getattr(self.django_settings, key, default)
+    def __setitem__(self, key, value):
+        setattr(settings, self._transform_key(key), value)
 
-    def __getattribute__(self, name):
-        if name == '_mapping':
-            return super(DjangoEnvironment, self).__getattribute__(name)
-        if not name in self._mapping:
-            return super(DjangoEnvironment, self).__getattribute__(name)
+    def __delitem__(self, key):
+        # This isn't possible to implement in Django without relying
+        # on internals of the settings object, so just set to None.
+        self.__setitem__(key, None)
 
-        django_key = self._mapping[name]
-        if not hasattr(self.django_settings, django_key):
-            return super(DjangoEnvironment, self).__getattribute__(name)
 
-        return getattr(self.django_settings, django_key)
+class DjangoEnvironment(Environment):
+    """For Django, we need to redirect all the configuration values this
+    object holds to Django's own settings object.
 
-    def __setattr__(self, name, value):
-        if not hasattr(self, 'django_settings'):
-            # The parent's __init__ is probably settings defaults
-            return super(DjangoEnvironment, self).__setattr__(name, value)
-        if not name in self._mapping:
-            return super(DjangoEnvironment, self).__setattr__(name, value)
+    We do this by hooking into __getattribute__ and __setattribute__,
+    rather than reimplementing the get_foo/set_foo() methods, which means
+    we won't have to reimplement any validation the parent class may do.
+    """
+
+    config_storage_class = DjangoConfigStorage
 
-        setattr(self.django_settings, self._mapping[name], value)
+    def __init__(self):
+        # Have the parent initialize the default values
+        super(DjangoEnvironment, self).__init__(settings.MEDIA_ROOT,
+                                                settings.MEDIA_URL)
 
 
 # Django has a global state, a global configuration, and so we need a

src/webassets/env.py

@@ -11,15 +11,76 @@ class RegisterError(Exception):
     pass
 
 
+class ConfigStorage(object):
+    """This is the backend which :class:`Environment` uses to store
+    it's configuration values.
+
+    Environment-subclasses like the one used by ``django-assets`` will
+    often want to use a custom ``ConfigStorage`` as well, building upon
+    whatever configuration the framework is using.
+
+    The goal in designing this class therefore is to make it easy for
+    subclasses to change the place the data is stored: Only
+    _meth:`__getitem__`, _meth:`__setitem__` and _meth:`__delitem__`
+    need to be implemented.
+
+    One rule: The default storage is case-insensitive, and custom
+    environments should maintain those semantics.
+
+    A related reason is why we don't inherit from ``dict``. It would
+    require us to re-implement a whole bunch of methods, like pop() etc.
+    """
+    # Don't inherit from ``dict``, as that would require us to implement
+    # a whole bunch of methods, like pop() etc.
+    def __init__(self, d={}):
+        self._dict = {}
+        self.update(d)
+
+    def __contains__(self, key):
+        return self._dict.__contains__(key.lower())
+
+    def get(self, key, default=None):
+        try:
+            return self.__getitem__(key)
+        except KeyError:
+            return default
+
+    def update(self, d):
+        for key in d:
+            self._dict[key] = d[key]
+
+    def __getitem__(self, key):
+        raise NotImplementedError()
+
+    def __setitem__(self, key, value):
+        raise NotImplementedError()
+
+    def __delitem__(self, key):
+        raise NotImplementedError()
+
+
+class DictConfigStorage(ConfigStorage):
+    """Using a lower-case dict for configuration values.
+    """
+    def __getitem__(self, key):
+        return self._dict.__getitem__(key.lower())
+    def __setitem__(self, key, value):
+        self._dict.__setitem__(key.lower(), value)
+    def __delitem__(self, key):
+        self._dict.__delitem__(key.lower())
+
+
 class Environment(object):
     """Owns a collection of bundles, and a set of configuration values
     which will be used when processing these bundles.
     """
 
+    config_storage_class = DictConfigStorage
+
     def __init__(self, directory, url, **config):
         self._named_bundles = {}
         self._anon_bundles = []
-        self._config = config
+        self._config = self.config_storage_class(config)
 
         self.directory = directory
         self.url = url
@@ -84,19 +145,18 @@ def add(self, *bundles):
             self._anon_bundles.append(bundle)
             bundle.env = self    # take ownership
 
-    def get_config(self, key, default=None):
-        """This is a simple configuration area provided by the asset
-        env which holds additional options used by the filters.
+    @property
+    def config(self):
+        """Key-value configuration. Keys are case-insensitive.
         """
-        return self._config.get(key.lower(), default)
-
-    def set_config(self, key, value):
-        self._config[key.lower()] = value
+        # This is a property so that user are not tempted to assign
+        # a custom dictionary which won't uphold our caseless semantics.
+        return self._config
 
     def set_debug(self, debug):
-        self._debug = debug
+        self.config['debug'] = debug
     def get_debug(self):
-        return self._debug
+        return self.config['debug']
     debug = property(get_debug, set_debug, doc=
     """Enable/disable debug mode. Possible values are:
 
@@ -110,9 +170,9 @@ def get_debug(self):
     """)
 
     def set_cache(self, enable):
-        self._cache = enable
+        self.config['cache'] = enable
     def get_cache(self):
-        return self._cache
+        return self.config['cache']
     cache = property(get_cache, set_cache, doc=
     """Controls the behavior of the cache. The cache will speed up rebuilding
     of your bundles, by caching individual filter results. This can be
@@ -135,9 +195,9 @@ def get_cache(self):
     """)
 
     def set_updater(self, updater):
-        self._updater = updater
+        self.config['updater'] = updater
     def get_updater(self):
-        return self._updater
+        return self.config['updater']
     updater = property(get_updater, set_updater, doc=
     """Controls when and if bundles should be automatically rebuilt.
     Possible values are:
@@ -167,9 +227,9 @@ def get_updater(self):
     """)
 
     def set_expire(self, expire):
-        self._expire = expire
+        self.config['expire'] = expire
     def get_expire(self):
-        return self._expire
+        return self.config['expire']
     expire = property(get_expire, set_expire, doc=
     """If you send your assets to the client using a *far future expires*
     header (to minimize the 304 responses your server has to send), you
@@ -193,17 +253,17 @@ def get_expire(self):
     """)
 
     def set_directory(self, directory):
-        self._directory = directory
+        self.config['directory'] = directory
     def get_directory(self):
-        return self._directory
+        return self.config['directory']
     directory = property(get_directory, set_directory, doc=
     """The base directory to which all paths will be relative to.
     """)
 
     def set_url(self, url):
-        self._url = url
+        self.config['url'] = url
     def get_url(self):
-        return self._url
+        return self.config['url']
     url = property(get_url, set_url, doc=
     """The base used to construct urls under which :attr:`directory`
     should be exposed.

src/webassets/filter/__init__.py

@@ -92,7 +92,7 @@ def get_config(self, setting=False, env=None, require=True,
 
         value = None
         if not setting is False:
-            value = self.env.get_config(setting, None)
+            value = self.env.config.get(setting, None)
 
         if value is None and not env is False:
             value = os.environ.get(env)

tests/test_environment.py

@@ -79,14 +79,14 @@ def setup(self):
         self.m = Environment(None, None)
 
     def test_basic(self):
-        assert self.m.get_config('foo') == None
-        self.m.set_config('foo', 'bar')
-        assert self.m.get_config('foo') == 'bar'
+        assert self.m.config.get('foo') == None
+        self.m.config['foo'] = 'bar'
+        assert self.m.config.get('foo') == 'bar'
 
     def test_case(self):
         """get_config() is case-insensitive.
         """
-        self.m.set_config('FoO', 'bar')
-        assert self.m.get_config('FOO') == 'bar'
-        assert self.m.get_config('foo') == 'bar'
-        assert self.m.get_config('fOO') == 'bar'
+        self.m.config['FoO'] = 'bar'
+        assert self.m.config.get('FOO') == 'bar'
+        assert self.m.config.get('foo') == 'bar'
+        assert self.m.config.get('fOO') == 'bar'

tests/test_ext/test_django.py

@@ -23,32 +23,29 @@ def setup_module():
     AssetsNode = Node
 
 
-def test_options():
-    """Various Environment options are backed by the Django settings
+class TestConfig(object):
+    """The environment configuration is backed by the Django settings
     object.
     """
-    settings.MEDIA_ROOT = 'FOO'
-    assert get_env().directory == 'FOO'
 
-    get_env().directory = 'BAR'
-    assert settings.MEDIA_ROOT == 'BAR'
+    def test_default_options(self):
+        """The builtin options have different names within the Django
+        settings, to make it obvious they belong to django-assets.
+        """
+        settings.ASSETS_EXPIRE = 'timestamp'
+        assert get_env().config['expire'] == settings.ASSETS_EXPIRE
 
-    # We can also access values that are not represented by a original
-    # Django setting. Specifically, we are able to read those values
-    # and get the webassets-default without having to explicitly
-    # initialize the corresponding Django setting.
-    assert get_env().debug == False
-    assert not hasattr(settings, 'ASSETS_DEBUG')
-    get_env().debug = True
-    assert settings.ASSETS_DEBUG == True
+        settings.MEDIA_ROOT = 'FOO'
+        assert get_env().directory == 'FOO'
 
+        get_env().directory = 'BAR'
+        assert settings.MEDIA_ROOT == 'BAR'
 
-def test_config():
-    """The Environment config storage is also backed by the Django
-    settings object.
-    """
-    settings.FOO = 42
-    get_env().get_config('FOO') == 42
+    def test_custom_options(self):
+        settings.FOO = 42
+        get_env().config['foo'] == 42
+        # Also, we are caseless.
+        get_env().config['foO'] == 42
 
 
 class TestTemplateTag():

tests/test_filters.py

@@ -47,7 +47,7 @@ def test_get_config(self):
             assert_raises(EnvironmentError, get_config, setting=NAME, env=False)
 
             # Set the value in the environment as well.
-            m.set_config(NAME, 'foo')
+            m.config[NAME] = 'foo'
             # Ensure that settings take precedence.
             assert_equals(get_config(NAME), 'foo')
             # Two different names can be supplied.