Skip to content

Commit

Permalink
Fixed override_settings when set_available_apps raises an exception.
Browse files Browse the repository at this point in the history
Previously, this would corrupt the settings, because __exit__ isn't
called when __enter__raises an exception.
  • Loading branch information
aaugustin committed Dec 23, 2013
1 parent 7577d03 commit 2ec8e34
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 15 deletions.
25 changes: 14 additions & 11 deletions django/apps/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def __init__(self, master=False):
# Cache for get_models.
self._get_models_cache = {}

def populate_apps(self):
def populate_apps(self, installed_apps=None):
"""
Populate app-related information.
Expand All @@ -77,7 +77,9 @@ def populate_apps(self):
# Application modules aren't expected to import anything, and
# especially not other application modules, even indirectly.
# Therefore we simply import them sequentially.
for app_name in settings.INSTALLED_APPS:
if installed_apps is None:
installed_apps = settings.INSTALLED_APPS
for app_name in installed_apps:
app_config = AppConfig.create(app_name)
self.app_configs[app_config.label] = app_config

Expand Down Expand Up @@ -299,6 +301,8 @@ def set_available_apps(self, available):
available must be an iterable of application names.
set_available_apps() must be balanced with unset_available_apps().
Primarily used for performance optimization in TransactionTestCase.
This method is safe is the sense that it doesn't trigger any imports.
Expand All @@ -323,10 +327,13 @@ def unset_available_apps(self):

def set_installed_apps(self, installed):
"""
Enables a different set of installed_apps for get_app_config[s].
Enables a different set of installed apps for get_app_config[s].
installed must be an iterable in the same format as INSTALLED_APPS.
set_installed_apps() must be balanced with unset_installed_apps(),
even if it exits with an exception.
Primarily used as a receiver of the setting_changed signal in tests.
This method may trigger new imports, which may add new models to the
Expand All @@ -337,14 +344,10 @@ def set_installed_apps(self, installed):
"""
self.stored_app_configs.append(self.app_configs)
self.app_configs = OrderedDict()
try:
self._apps_loaded = False
self.populate_apps()
self._models_loaded = False
self.populate_models()
except Exception:
self.unset_installed_apps()
raise
self._apps_loaded = False
self.populate_apps(installed)
self._models_loaded = False
self.populate_models()

def unset_installed_apps(self):
"""
Expand Down
14 changes: 10 additions & 4 deletions django/test/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,22 +225,28 @@ def save_options(self, test_func):
test_func._overridden_settings, **self.options)

def enable(self):
# Keep this code at the beginning to leave the settings unchanged
# in case it raises an exception because INSTALLED_APPS is invalid.
if 'INSTALLED_APPS' in self.options:
try:
app_cache.set_installed_apps(self.options['INSTALLED_APPS'])
except Exception:
app_cache.unset_installed_apps()
raise
override = UserSettingsHolder(settings._wrapped)
for key, new_value in self.options.items():
setattr(override, key, new_value)
self.wrapped = settings._wrapped
settings._wrapped = override
if 'INSTALLED_APPS' in self.options:
app_cache.set_installed_apps(settings.INSTALLED_APPS)
for key, new_value in self.options.items():
setting_changed.send(sender=settings._wrapped.__class__,
setting=key, value=new_value, enter=True)

def disable(self):
settings._wrapped = self.wrapped
del self.wrapped
if 'INSTALLED_APPS' in self.options:
app_cache.unset_installed_apps()
settings._wrapped = self.wrapped
del self.wrapped
for key in self.options:
new_value = getattr(settings, key, None)
setting_changed.send(sender=settings._wrapped.__class__,
Expand Down

0 comments on commit 2ec8e34

Please sign in to comment.