From 88a2d39159872f6a7fced1bae591550650fd7f38 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Mon, 27 Jan 2014 13:28:53 -0700 Subject: [PATCH] Fixed #21874 -- Require Django applications to have a filesystem path. Wherever possible this filesystem path is derived automatically from the app module's ``__path__`` and ``__file__`` attributes (this avoids any backwards-compatibility problems). AppConfig allows specifying an app's filesystem location explicitly, which overrides all autodetection based on ``__path__`` and ``__file__``. This permits Django to support any type of module as an app (namespace packages, fake modules, modules loaded by other hypothetical non-filesystem module loaders), as long as the app is configured with an explicit filesystem path. Thanks Aymeric for review and discussion. --- django/apps/config.py | 43 +++++++++++------- django/db/migrations/state.py | 5 ++- docs/ref/applications.txt | 2 - tests/apps/tests.py | 67 +++++++++++++++++++++++++++- tests/template_tests/test_loaders.py | 5 +++ 5 files changed, 102 insertions(+), 20 deletions(-) diff --git a/django/apps/config.py b/django/apps/config.py index 1334da4c03..f7144505df 100644 --- a/django/apps/config.py +++ b/django/apps/config.py @@ -1,4 +1,5 @@ from importlib import import_module +import os from django.core.exceptions import ImproperlyConfigured from django.utils.module_loading import module_has_submodule @@ -34,23 +35,10 @@ class AppConfig(object): self.verbose_name = self.label.title() # Filesystem path to the application directory eg. - # u'/usr/lib/python2.7/dist-packages/django/contrib/admin'. May be - # None if the application isn't a bona fide package eg. if it's an - # egg. Otherwise it's a unicode on Python 2 and a str on Python 3. + # u'/usr/lib/python2.7/dist-packages/django/contrib/admin'. Unicode on + # Python 2 and a str on Python 3. if not hasattr(self, 'path'): - try: - paths = app_module.__path__ - except AttributeError: - self.path = None - else: - # Convert paths to list because Python 3.3 _NamespacePath does - # not support indexing. - paths = list(paths) - if len(paths) > 1: - raise ImproperlyConfigured( - "The namespace package app %r has multiple locations, " - "which is not supported: %r" % (app_name, paths)) - self.path = upath(paths[0]) + self.path = self._path_from_module(app_module) # Module containing models eg. . Set by import_models(). @@ -64,6 +52,29 @@ class AppConfig(object): def __repr__(self): return '<%s: %s>' % (self.__class__.__name__, self.label) + def _path_from_module(self, module): + """Attempt to determine app's filesystem path from its module.""" + # See #21874 for extended discussion of the behavior of this method in + # various cases. + # Convert paths to list because Python 3.3 _NamespacePath does not + # support indexing. + paths = list(getattr(module, '__path__', [])) + if len(paths) != 1: + filename = getattr(module, '__file__', None) + if filename is not None: + paths = [os.path.dirname(filename)] + if len(paths) > 1: + raise ImproperlyConfigured( + "The app module %r has multiple filesystem locations (%r); " + "you must configure this app with an AppConfig subclass " + "with a 'path' class attribute." % (module, paths)) + elif not paths: + raise ImproperlyConfigured( + "The app module %r has no filesystem location, " + "you must configure this app with an AppConfig subclass " + "with a 'path' class attribute." % (module,)) + return upath(paths[0]) + @classmethod def create(cls, entry): """ diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py index f802b6cc0b..949d2251e5 100644 --- a/django/db/migrations/state.py +++ b/django/db/migrations/state.py @@ -73,8 +73,11 @@ class ProjectState(object): class AppConfigStub(AppConfig): """ - Stubs a Django AppConfig. Only provides a label and a dict of models. + Stubs a Django AppConfig. Only provides a label, and a dict of models. """ + # Not used, but required by AppConfig.__init__ + path = '' + def __init__(self, label): super(AppConfigStub, self).__init__(label, None) diff --git a/docs/ref/applications.txt b/docs/ref/applications.txt index df105f1237..60db009585 100644 --- a/docs/ref/applications.txt +++ b/docs/ref/applications.txt @@ -171,8 +171,6 @@ Configurable attributes required; for instance if the app package is a `namespace package`_ with multiple paths. - It may be ``None`` if the application isn't stored in a directory. - Read-only attributes -------------------- diff --git a/tests/apps/tests.py b/tests/apps/tests.py index d09757c7bd..777e26d816 100644 --- a/tests/apps/tests.py +++ b/tests/apps/tests.py @@ -4,7 +4,7 @@ import os import sys from unittest import skipUnless -from django.apps import apps +from django.apps import apps, AppConfig from django.apps.registry import Apps from django.contrib.admin.models import LogEntry from django.core.exceptions import ImproperlyConfigured @@ -201,6 +201,71 @@ class AppsTests(TestCase): self.assertEqual(new_apps.get_model("apps", "SouthPonies"), temp_model) +class Stub(object): + def __init__(self, **kwargs): + self.__dict__.update(kwargs) + + +class AppConfigTests(TestCase): + """Unit tests for AppConfig class.""" + def test_path_set_explicitly(self): + """If subclass sets path as class attr, no module attributes needed.""" + class MyAppConfig(AppConfig): + path = 'foo' + + ac = MyAppConfig('label', Stub()) + + self.assertEqual(ac.path, 'foo') + + def test_explicit_path_overrides(self): + """If path set as class attr, overrides __path__ and __file__.""" + class MyAppConfig(AppConfig): + path = 'foo' + + ac = MyAppConfig('label', Stub(__path__=['a'], __file__='b/__init__.py')) + + self.assertEqual(ac.path, 'foo') + + def test_dunder_path(self): + """If single element in __path__, use it (in preference to __file__).""" + ac = AppConfig('label', Stub(__path__=['a'], __file__='b/__init__.py')) + + self.assertEqual(ac.path, 'a') + + def test_no_dunder_path_fallback_to_dunder_file(self): + """If there is no __path__ attr, use __file__.""" + ac = AppConfig('label', Stub(__file__='b/__init__.py')) + + self.assertEqual(ac.path, 'b') + + def test_empty_dunder_path_fallback_to_dunder_file(self): + """If the __path__ attr is empty, use __file__ if set.""" + ac = AppConfig('label', Stub(__path__=[], __file__='b/__init__.py')) + + self.assertEqual(ac.path, 'b') + + def test_multiple_dunder_path_fallback_to_dunder_file(self): + """If the __path__ attr is length>1, use __file__ if set.""" + ac = AppConfig('label', Stub(__path__=['a', 'b'], __file__='c/__init__.py')) + + self.assertEqual(ac.path, 'c') + + def test_no_dunder_path_or_dunder_file(self): + """If there is no __path__ or __file__, raise ImproperlyConfigured.""" + with self.assertRaises(ImproperlyConfigured): + AppConfig('label', Stub()) + + def test_empty_dunder_path_no_dunder_file(self): + """If the __path__ attr is empty and there is no __file__, raise.""" + with self.assertRaises(ImproperlyConfigured): + AppConfig('label', Stub(__path__=[])) + + def test_multiple_dunder_path_no_dunder_file(self): + """If the __path__ attr is length>1 and there is no __file__, raise.""" + with self.assertRaises(ImproperlyConfigured): + AppConfig('label', Stub(__path__=['a', 'b'])) + + @skipUnless( sys.version_info > (3, 3, 0), "Namespace packages sans __init__.py were added in Python 3.3") diff --git a/tests/template_tests/test_loaders.py b/tests/template_tests/test_loaders.py index 6c16b9bbbe..ea582aef38 100644 --- a/tests/template_tests/test_loaders.py +++ b/tests/template_tests/test_loaders.py @@ -43,6 +43,8 @@ def create_egg(name, resources): """ egg = types.ModuleType(name) egg.__loader__ = MockLoader() + egg.__path__ = ['/some/bogus/path/'] + egg.__file__ = '/some/bogus/path/__init__.pyc' egg._resources = resources sys.modules[name] = egg @@ -68,6 +70,9 @@ class EggLoaderTest(TestCase): def _get(self, path): return self.module._resources[path].read() + def _fn(self, base, resource_name): + return resource_name + pkg_resources._provider_factories[MockLoader] = MockProvider self.empty_egg = create_egg("egg_empty", {})