From 6feb75129f42fdac751d0b79a2a005b4c0ad5c2d Mon Sep 17 00:00:00 2001 From: Juan Catalano Date: Fri, 6 Sep 2013 20:23:25 -0300 Subject: [PATCH] Fixed #21060 -- Refactored admin's autodiscover method to make it reusable. We want to be able to use it for instance for discovering `tasks.py` modules inside the INSTALLED_APPS. This commit therefore moves the logic to `autodiscover_modules` method in django.utils.module_loading. --- django/contrib/admin/__init__.py | 31 +-------------- django/utils/module_loading.py | 38 +++++++++++++++++++ tests/utils_tests/test_module/__init__.py | 3 ++ .../test_module/another_bad_module.py | 8 ++++ .../test_module/another_good_module.py | 1 + tests/utils_tests/test_module_loading.py | 31 ++++++++++++++- 6 files changed, 82 insertions(+), 30 deletions(-) create mode 100644 tests/utils_tests/test_module/another_bad_module.py create mode 100644 tests/utils_tests/test_module/another_good_module.py diff --git a/django/contrib/admin/__init__.py b/django/contrib/admin/__init__.py index ef2e87407ad..39759936799 100644 --- a/django/contrib/admin/__init__.py +++ b/django/contrib/admin/__init__.py @@ -7,35 +7,8 @@ from django.contrib.admin.sites import AdminSite, site from django.contrib.admin.filters import (ListFilter, SimpleListFilter, FieldListFilter, BooleanFieldListFilter, RelatedFieldListFilter, ChoicesFieldListFilter, DateFieldListFilter, AllValuesFieldListFilter) +from django.utils.module_loading import autodiscover_modules def autodiscover(): - """ - Auto-discover INSTALLED_APPS admin.py modules and fail silently when - not present. This forces an import on them to register any admin bits they - may want. - """ - - import copy - from importlib import import_module - from django.conf import settings - from django.utils.module_loading import module_has_submodule - - for app in settings.INSTALLED_APPS: - mod = import_module(app) - # Attempt to import the app's admin module. - try: - before_import_registry = copy.copy(site._registry) - import_module('%s.admin' % app) - except: - # Reset the model registry to the state before the last import as - # this import will have to reoccur on the next request and this - # could raise NotRegistered and AlreadyRegistered exceptions - # (see #8245). - site._registry = before_import_registry - - # Decide whether to bubble up this error. If the app just - # doesn't have an admin module, we can ignore the error - # attempting to import it, otherwise we want it to bubble up. - if module_has_submodule(mod, 'admin'): - raise + autodiscover_modules('admin', register_to=site) diff --git a/django/utils/module_loading.py b/django/utils/module_loading.py index 9c8ea98d509..8868a2d3ab3 100644 --- a/django/utils/module_loading.py +++ b/django/utils/module_loading.py @@ -1,5 +1,6 @@ from __future__ import absolute_import # Avoid importing `importlib` from this package. +import copy import imp from importlib import import_module import os @@ -34,6 +35,43 @@ def import_by_path(dotted_path, error_prefix=''): return attr +def autodiscover_modules(*args, **kwargs): + """ + Auto-discover INSTALLED_APPS modules and fail silently when + not present. This forces an import on them to register any admin bits they + may want. + + You may provide a register_to keyword parameter as a way to access a + registry. This register_to object must have a _registry instance variable + to access it. + """ + from django.conf import settings + + register_to = kwargs.get('register_to') + for app in settings.INSTALLED_APPS: + mod = import_module(app) + # Attempt to import the app's module. + try: + if register_to: + before_import_registry = copy.copy(register_to._registry) + + for module_to_search in args: + import_module('%s.%s' % (app, module_to_search)) + except: + # Reset the model registry to the state before the last import as + # this import will have to reoccur on the next request and this + # could raise NotRegistered and AlreadyRegistered exceptions + # (see #8245). + if register_to: + register_to._registry = before_import_registry + + # Decide whether to bubble up this error. If the app just + # doesn't have an admin module, we can ignore the error + # attempting to import it, otherwise we want it to bubble up. + if module_has_submodule(mod, module_to_search): + raise + + def module_has_submodule(package, module_name): """See if 'module' is in 'package'.""" name = ".".join([package.__name__, module_name]) diff --git a/tests/utils_tests/test_module/__init__.py b/tests/utils_tests/test_module/__init__.py index e69de29bb2d..8f33921eb66 100644 --- a/tests/utils_tests/test_module/__init__.py +++ b/tests/utils_tests/test_module/__init__.py @@ -0,0 +1,3 @@ +class SiteMock(object): + _registry = {} +site = SiteMock() diff --git a/tests/utils_tests/test_module/another_bad_module.py b/tests/utils_tests/test_module/another_bad_module.py new file mode 100644 index 00000000000..eac25c4aa56 --- /dev/null +++ b/tests/utils_tests/test_module/another_bad_module.py @@ -0,0 +1,8 @@ +from . import site +content = 'Another Bad Module' + +site._registry.update({ + 'foo': 'bar', +}) + +raise Exception('Some random exception.') diff --git a/tests/utils_tests/test_module/another_good_module.py b/tests/utils_tests/test_module/another_good_module.py new file mode 100644 index 00000000000..ed4b2d34a28 --- /dev/null +++ b/tests/utils_tests/test_module/another_good_module.py @@ -0,0 +1 @@ +content = 'Another Good Module' diff --git a/tests/utils_tests/test_module_loading.py b/tests/utils_tests/test_module_loading.py index 2423215778c..d808ab87837 100644 --- a/tests/utils_tests/test_module_loading.py +++ b/tests/utils_tests/test_module_loading.py @@ -6,7 +6,10 @@ import unittest from zipimport import zipimporter from django.core.exceptions import ImproperlyConfigured -from django.utils.module_loading import import_by_path, module_has_submodule +from django.test import SimpleTestCase +from django.test.utils import override_settings +from django.utils import six +from django.utils.module_loading import autodiscover_modules, import_by_path, module_has_submodule from django.utils._os import upath @@ -130,6 +133,32 @@ class ModuleImportTestCase(unittest.TestCase): self.assertIsNotNone(traceback.tb_next.tb_next, 'Should have more than the calling frame in the traceback.') +@override_settings(INSTALLED_APPS=('utils_tests.test_module',)) +class AutodiscoverModulesTestCase(SimpleTestCase): + + def test_autodiscover_modules_found(self): + autodiscover_modules('good_module') + + def test_autodiscover_modules_not_found(self): + autodiscover_modules('missing_module') + + def test_autodiscover_modules_found_but_bad_module(self): + with six.assertRaisesRegex(self, ImportError, "No module named '?a_package_name_that_does_not_exist'?"): + autodiscover_modules('bad_module') + + def test_autodiscover_modules_several_one_bad_module(self): + with six.assertRaisesRegex(self, ImportError, "No module named '?a_package_name_that_does_not_exist'?"): + autodiscover_modules('good_module', 'bad_module') + + def test_autodiscover_modules_several_found(self): + autodiscover_modules('good_module', 'another_good_module') + + def test_validate_registry_keeps_intact(self): + from .test_module import site + with six.assertRaisesRegex(self, Exception, "Some random exception."): + autodiscover_modules('another_bad_module', register_to=site) + self.assertEqual(site._registry, {}) + class ProxyFinder(object): def __init__(self):