Fixed #13348: Restored ability to load models from apps in eggs. Thanks Ramiro and metzen for pointers on how to find out if a module loaded from an egg has a particular submodule.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@12982 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/db/models/loading.py

@@ -4,6 +4,7 @@ from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 from django.utils.datastructures import SortedDict
 from django.utils.importlib import import_module
+from django.utils.module_loading import module_has_submodule
 
 import imp
 import sys
@@ -73,25 +74,28 @@ class AppCache(object):
         self.handled[app_name] = None
         self.nesting_level += 1
         app_module = import_module(app_name)
-        try:
-            imp.find_module('models', app_module.__path__)
-        except ImportError: 
-            self.nesting_level -= 1
-            # App has no models module, that's not a problem.
-            return None
         try:
             models = import_module('.models', app_name)
         except ImportError:
             self.nesting_level -= 1
-            if can_postpone:
+            # If the app doesn't have a models module, we can just ignore the
-                # Either the app has an error, or the package is still being
+            # ImportError and return no models for it.
-                # imported by Python and the model module isn't available yet.
+            if not module_has_submodule(app_module, 'models'):
-                # We will check again once all the recursion has finished (in
-                # populate).
-                self.postponed.append(app_name)
                 return None
+            # But if the app does have a models module, we need to figure out
+            # whether to suppress or propagate the error. If can_postpone is
+            # True then it may be that the package is still being imported by
+            # Python and the models module isn't available yet. So we add the
+            # app to the postponed list and we'll try it again after all the
+            # recursion has finished (in populate). If can_postpone is False
+            # then it's time to raise the ImportError.
             else:
-                raise
+                if can_postpone:
+                    self.postponed.append(app_name)
+                    return None
+                else:
+                    raise
+
         self.nesting_level -= 1
         if models not in self.app_store:
             self.app_store[models] = len(self.app_store)

django/utils/module_loading.py

@@ -0,0 +1,27 @@
+import os
+import imp
+
+def module_has_submodule(mod, submod_name):
+    # If the module was loaded from an egg, __loader__ will be set and 
+    # its find_module must be used to search for submodules.
+    loader = getattr(mod, '__loader__', None)
+    if loader:
+        mod_path = "%s.%s" % (mod.__name__, submod_name)
+        mod_path = mod_path[len(loader.prefix):]
+        x = loader.find_module(mod_path)
+        if x is None:
+            # zipimport.zipimporter.find_module is documented to take
+            # dotted paths but in fact through Pyton 2.7 is observed 
+            # to require os.sep in place of dots...so try using os.sep
+            # if the dotted path version failed to find the requested 
+            # submodule.
+            x = loader.find_module(mod_path.replace('.', os.sep))
+        return x is not None
+
+    try:
+        imp.find_module(submod_name, mod.__path__)
+        return True
+    except ImportError:
+        return False
+
+

tests/regressiontests/app_loading/tests.py

@@ -1,8 +1,10 @@
 import os
 import sys
 import time
+from unittest import TestCase
 
 from django.conf import Settings
+from django.db.models.loading import load_app
 
 __test__ = {"API_TESTS": """
 Test the globbing of INSTALLED_APPS.
@@ -25,3 +27,51 @@ Test the globbing of INSTALLED_APPS.
 
 """}
 
+class EggLoadingTest(TestCase):
+
+    def setUp(self):
+        self.old_path = sys.path
+        self.egg_dir = '%s/eggs' % os.path.dirname(__file__)
+
+    def tearDown(self):
+        sys.path = self.old_path
+
+    def test_egg1(self):
+        """Models module can be loaded from an app in an egg"""
+        egg_name = '%s/modelapp.egg' % self.egg_dir
+        sys.path.append(egg_name)
+        models = load_app('app_with_models')
+        self.failIf(models is None)
+
+    def test_egg2(self):
+        """Loading an app from an egg that has no models returns no models (and no error)"""
+        egg_name = '%s/nomodelapp.egg' % self.egg_dir
+        sys.path.append(egg_name)
+        models = load_app('app_no_models')
+        self.failUnless(models is None)
+
+    def test_egg3(self):
+        """Models module can be loaded from an app located under an egg's top-level package"""
+        egg_name = '%s/omelet.egg' % self.egg_dir
+        sys.path.append(egg_name)
+        models = load_app('omelet.app_with_models')
+        self.failIf(models is None)
+
+    def test_egg4(self):
+        """Loading an app with no models from under the top-level egg package generates no error"""
+        egg_name = '%s/omelet.egg' % self.egg_dir
+        sys.path.append(egg_name)
+        models = load_app('omelet.app_no_models')
+        self.failUnless(models is None)
+ 
+    def test_egg5(self):
+        """Loading an app from an egg that has an import error in its models module raises that error"""
+        egg_name = '%s/brokenapp.egg' % self.egg_dir
+        sys.path.append(egg_name)
+        self.assertRaises(ImportError, load_app, 'broken_app')
+        try:
+            load_app('broken_app')
+        except ImportError, e:
+            # Make sure the message is indicating the actual
+            # problem in the broken app.
+            self.failUnless("modelz" in e.args[0])