From a4cb1400046f29f933b388ad6390f517e1f80c7a Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Fri, 13 Dec 2013 12:02:54 +0100 Subject: [PATCH] Added get_app_config() to look up app configs by label. Refactored get_app() to rely on that method. get_app() starts by calling _populate(), which goes through INSTALLED_APPS and, for each app, imports the app module and attempts to import the models module. At this point, no further imports are necessary to return the models module for a given app. Therefore, the implementation of get_app() can be simplified and the safeguards for race conditions can be removed. Besides, the emptyOK parameter isn't used anywhere in Django. It was introduced in d6c95e93 but not actually used nor documented, and it has just been carried around since then. Since it's an obscure private API, it's acceptable to stop supporting it without a deprecation path. This branch aims at providing first-class support for applications without a models module eventually. For backwards-compatibility, get_app() still raises ImproperlyConfigured when an app isn't found, even though LookupError is technically more correct. I haven't gone as far as to preserve the exact error messages. I've adjusted a few tests instead. --- django/apps/cache.py | 41 ++++++++++++++++++++++-------------- tests/admin_scripts/tests.py | 16 +++++++------- tests/empty/tests.py | 2 +- 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/django/apps/cache.py b/django/apps/cache.py index 182d89cc01..893b1af2b4 100644 --- a/django/apps/cache.py +++ b/django/apps/cache.py @@ -153,6 +153,26 @@ class BaseAppCache(object): """ return self.loaded + def get_app_config(self, app_label, only_installed=True): + """ + Returns the application configuration for the given app_label. + + Raises LookupError if no application exists with this app_label. + + Raises UnavailableApp when set_available_apps() disables the + application with this app_label. + + If only_installed is True (default), only applications explicitly + listed in INSTALLED_APPS are considered. + """ + self._populate() + app_config = self.app_configs.get(app_label) + if app_config is None or (only_installed and not app_config.installed): + raise LookupError("No app with label %r." % app_label) + if self.available_apps is not None and app_config.label not in self.available_apps: + raise UnavailableApp("App with label %r isn't available." % app_label) + return app_config + def get_apps(self): """ Returns a list of all installed modules that contain models. @@ -197,29 +217,18 @@ class BaseAppCache(object): app_paths.append(self._get_app_path(app)) return app_paths - def get_app(self, app_label, emptyOK=False): + def get_app(self, app_label): """ Returns the module containing the models for the given app_label. - Returns None if the app has no models in it and emptyOK is True. - Raises UnavailableApp when set_available_apps() in in effect and doesn't include app_label. """ - self._populate() - imp.acquire_lock() try: - for app_name in settings.INSTALLED_APPS: - if app_label == app_name.split('.')[-1]: - mod = self.load_app(app_name, False) - if mod is None and not emptyOK: - raise ImproperlyConfigured("App with label %s is missing a models.py module." % app_label) - if self.available_apps is not None and app_label not in self.available_apps: - raise UnavailableApp("App with label %s isn't available." % app_label) - return mod - raise ImproperlyConfigured("App with label %s could not be found" % app_label) - finally: - imp.release_lock() + return self.get_app_config(app_label).models_module + except LookupError as exc: + # Change the exception type for backwards compatibility. + raise ImproperlyConfigured(*exc.args) def get_models(self, app_mod=None, include_auto_created=False, include_deferred=False, diff --git a/tests/admin_scripts/tests.py b/tests/admin_scripts/tests.py index 73a91b6e7b..8f693bfd34 100644 --- a/tests/admin_scripts/tests.py +++ b/tests/admin_scripts/tests.py @@ -379,14 +379,14 @@ class DjangoAdminMinimalSettings(AdminScriptTestCase): args = ['sqlall', '--settings=test_project.settings', 'admin_scripts'] out, err = self.run_django_admin(args) self.assertNoOutput(out) - self.assertOutput(err, 'App with label admin_scripts could not be found') + self.assertOutput(err, "No app with label 'admin_scripts'.") def test_builtin_with_environment(self): "minimal: django-admin builtin commands fail if settings are provided in the environment" args = ['sqlall', 'admin_scripts'] out, err = self.run_django_admin(args, 'test_project.settings') self.assertNoOutput(out) - self.assertOutput(err, 'App with label admin_scripts could not be found') + self.assertOutput(err, "No app with label 'admin_scripts'.") def test_builtin_with_bad_settings(self): "minimal: django-admin builtin commands fail if settings file (from argument) doesn't exist" @@ -815,21 +815,21 @@ class ManageMinimalSettings(AdminScriptTestCase): args = ['sqlall', 'admin_scripts'] out, err = self.run_manage(args) self.assertNoOutput(out) - self.assertOutput(err, 'App with label admin_scripts could not be found') + self.assertOutput(err, "No app with label 'admin_scripts'.") def test_builtin_with_settings(self): "minimal: manage.py builtin commands fail if settings are provided as argument" args = ['sqlall', '--settings=test_project.settings', 'admin_scripts'] out, err = self.run_manage(args) self.assertNoOutput(out) - self.assertOutput(err, 'App with label admin_scripts could not be found') + self.assertOutput(err, "No app with label 'admin_scripts'.") def test_builtin_with_environment(self): "minimal: manage.py builtin commands fail if settings are provided in the environment" args = ['sqlall', 'admin_scripts'] out, err = self.run_manage(args, 'test_project.settings') self.assertNoOutput(out) - self.assertOutput(err, 'App with label admin_scripts could not be found') + self.assertOutput(err, "No app with label 'admin_scripts'.") def test_builtin_with_bad_settings(self): "minimal: manage.py builtin commands fail if settings file (from argument) doesn't exist" @@ -964,7 +964,7 @@ class ManageMultipleSettings(AdminScriptTestCase): args = ['sqlall', 'admin_scripts'] out, err = self.run_manage(args) self.assertNoOutput(out) - self.assertOutput(err, 'App with label admin_scripts could not be found.') + self.assertOutput(err, "No app with label 'admin_scripts'.") def test_builtin_with_settings(self): "multiple: manage.py builtin commands succeed if settings are provided as argument" @@ -1442,13 +1442,13 @@ class CommandTypes(AdminScriptTestCase): "User AppCommands can execute when a single app name is provided" args = ['app_command', 'NOT_AN_APP'] out, err = self.run_manage(args) - self.assertOutput(err, "App with label NOT_AN_APP could not be found") + self.assertOutput(err, "No app with label 'NOT_AN_APP'.") def test_app_command_some_invalid_appnames(self): "User AppCommands can execute when some of the provided app names are invalid" args = ['app_command', 'auth', 'NOT_AN_APP'] out, err = self.run_manage(args) - self.assertOutput(err, "App with label NOT_AN_APP could not be found") + self.assertOutput(err, "No app with label 'NOT_AN_APP'.") def test_label_command(self): "User LabelCommands can execute when a label is provided" diff --git a/tests/empty/tests.py b/tests/empty/tests.py index b8476fc73d..824c1f16e7 100644 --- a/tests/empty/tests.py +++ b/tests/empty/tests.py @@ -32,5 +32,5 @@ class NoModelTests(TestCase): @override_settings(INSTALLED_APPS=("empty.no_models",)) def test_no_models(self): with six.assertRaisesRegex(self, ImproperlyConfigured, - 'App with label no_models is missing a models.py module.'): + "No app with label 'no_models'."): app_cache.get_app('no_models')