Stopped staticfiles app from requiring a models module when looking for static files. Also removed a bit of code smell in the prefix handling by saving it in the source file storage.

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

django/contrib/staticfiles/finders.py

@@ -55,7 +55,9 @@ class FileSystemFinder(BaseFinder):
             self.locations.add((prefix, root))
         # Don't initialize multiple storages for the same location
         for prefix, root in self.locations:
-            self.storages[root] = FileSystemStorage(location=root)
+            filesystem_storage = FileSystemStorage(location=root)
+            filesystem_storage.prefix = prefix
+            self.storages[root] = filesystem_storage
 
         super(FileSystemFinder, self).__init__(*args, **kwargs)
 
@@ -94,7 +96,7 @@ class FileSystemFinder(BaseFinder):
         for prefix, root in self.locations:
             storage = self.storages[root]
             for path in utils.get_files(storage, ignore_patterns):
-                yield path, prefix, storage
+                yield path, storage
 
 
 class AppDirectoriesFinder(BaseFinder):
@@ -105,14 +107,18 @@ class AppDirectoriesFinder(BaseFinder):
     storage_class = AppStaticStorage
 
     def __init__(self, apps=None, *args, **kwargs):
-        # Maps app modules to appropriate storage instances
+        # The list of apps that are handled
+        self.apps = []
+        # Mapping of app module paths to storage instances
         self.storages = SortedDict()
-        if apps is not None:
+        if apps is None:
-            self.apps = apps
+            apps = settings.INSTALLED_APPS
-        else:
+        for app in apps:
-            self.apps = models.get_apps()
+            app_storage = self.storage_class(app)
-        for app in self.apps:
+            if os.path.isdir(app_storage.location):
-            self.storages[app] = self.storage_class(app)
+                self.storages[app] = app_storage
+                if app not in self.apps:
+                    self.apps.append(app)
         super(AppDirectoriesFinder, self).__init__(*args, **kwargs)
 
     def list(self, ignore_patterns):
@@ -121,9 +127,8 @@ class AppDirectoriesFinder(BaseFinder):
         """
         for storage in self.storages.itervalues():
             if storage.exists(''): # check if storage location exists
-                prefix = storage.get_prefix()
                 for path in utils.get_files(storage, ignore_patterns):
-                    yield path, prefix, storage
+                    yield path, storage
 
     def find(self, path, all=False):
         """
@@ -131,29 +136,29 @@ class AppDirectoriesFinder(BaseFinder):
         """
         matches = []
         for app in self.apps:
-            app_matches = self.find_in_app(app, path)
+            match = self.find_in_app(app, path)
-            if app_matches:
+            if match:
                 if not all:
-                    return app_matches
+                    return match
-                matches.append(app_matches)
+                matches.append(match)
         return matches
 
     def find_in_app(self, app, path):
         """
         Find a requested static file in an app's static locations.
         """
-        storage = self.storages[app]
+        storage = self.storages.get(app, None)
-        prefix = storage.get_prefix()
+        if storage:
-        if prefix:
+            if storage.prefix:
-            prefix = '%s%s' % (prefix, os.sep)
+                prefix = '%s%s' % (storage.prefix, os.sep)
-            if not path.startswith(prefix):
+                if not path.startswith(prefix):
-                return None
+                    return None
-            path = path[len(prefix):]
+                path = path[len(prefix):]
-        # only try to find a file if the source dir actually exists
+            # only try to find a file if the source dir actually exists
-        if storage.exists(path):
+            if storage.exists(path):
-            matched_path = storage.path(path)
+                matched_path = storage.path(path)
-            if matched_path:
+                if matched_path:
-                return matched_path
+                    return matched_path
 
 
 class BaseStorageFinder(BaseFinder):
@@ -196,7 +201,7 @@ class BaseStorageFinder(BaseFinder):
         List all files of the storage.
         """
         for path in utils.get_files(self.storage, ignore_patterns):
-            yield path, '', self.storage
+            yield path, self.storage
 
 class DefaultStorageFinder(BaseStorageFinder):
     """

django/contrib/staticfiles/management/commands/collectstatic.py

@@ -75,8 +75,8 @@ Type 'yes' to continue, or 'no' to cancel: """)
                 raise CommandError("Collecting static files cancelled.")
 
         for finder in finders.get_finders():
-            for source, prefix, storage in finder.list(ignore_patterns):
+            for source, storage in finder.list(ignore_patterns):
-                self.copy_file(source, prefix, storage, **options)
+                self.copy_file(source, storage, **options)
 
         actual_count = len(self.copied_files) + len(self.symlinked_files)
         unmodified_count = len(self.unmodified_files)
@@ -97,7 +97,7 @@ Type 'yes' to continue, or 'no' to cancel: """)
         if self.verbosity >= level:
             self.stdout.write(msg)
 
-    def copy_file(self, source, prefix, source_storage, **options):
+    def copy_file(self, source, source_storage, **options):
         """
         Attempt to copy (or symlink) ``source`` to ``destination``,
         returning True if successful.
@@ -107,8 +107,8 @@ Type 'yes' to continue, or 'no' to cancel: """)
             source_last_modified = source_storage.modified_time(source)
         except (OSError, NotImplementedError):
             source_last_modified = None
-        if prefix:
+        if getattr(source_storage, 'prefix', None):
-            destination = os.path.join(prefix, source)
+            destination = os.path.join(source_storage.prefix, source)
         else:
             destination = source
         symlink = options['link']

django/contrib/staticfiles/storage.py

@@ -38,50 +38,22 @@ class AppStaticStorage(FileSystemStorage):
     A file system storage backend that takes an app module and works
     for the ``static`` directory of it.
     """
+    prefix = None
     source_dir = 'static'
 
     def __init__(self, app, *args, **kwargs):
         """
         Returns a static file storage if available in the given app.
         """
-        # app is actually the models module of the app. Remove the '.models'.
+        # app is the actual app module
-        bits = app.__name__.split('.')[:-1]
+        self.app_module = app
-        self.app_name = bits[-1]
+        # We special case the admin app here since it has its static files
-        self.app_module = '.'.join(bits)
+        # in 'media' for historic reasons.
-        # The models module (app) may be a package in which case
+        if self.app_module == 'django.contrib.admin':
-        # dirname(app.__file__) would be wrong. Import the actual app
+            self.prefix = 'admin'
-        # as opposed to the models module.
+            self.source_dir = 'media'
-        app = import_module(self.app_module)
+        mod = import_module(self.app_module)
-        location = self.get_location(os.path.dirname(app.__file__))
+        mod_path = os.path.dirname(mod.__file__)
+        location = os.path.join(mod_path, self.source_dir)
         super(AppStaticStorage, self).__init__(location, *args, **kwargs)
 
-    def get_location(self, app_root):
-        """
-        Given the app root, return the location of the static files of an app,
-        by default 'static'. We special case the admin app here since it has
-        its static files in 'media'.
-        """
-        if self.app_module == 'django.contrib.admin':
-            return os.path.join(app_root, 'media')
-        return os.path.join(app_root, self.source_dir)
-
-    def get_prefix(self):
-        """
-        Return the path name that should be prepended to files for this app.
-        """
-        if self.app_module == 'django.contrib.admin':
-            return self.app_name
-        return None
-
-    def get_files(self, ignore_patterns=[]):
-        """
-        Return a list containing the relative source paths for all files that
-        should be copied for an app.
-        """
-        files = []
-        prefix = self.get_prefix()
-        for path in utils.get_files(self, ignore_patterns):
-            if prefix:
-                path = os.path.join(prefix, path)
-            files.append(path)
-        return files

django/contrib/staticfiles/utils.py

@@ -3,32 +3,35 @@ import fnmatch
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 
+def is_ignored(path, ignore_patterns=[]):
+    """
+    Return True or False depending on whether the ``path`` should be
+    ignored (if it matches any pattern in ``ignore_patterns``).
+    """
+    for pattern in ignore_patterns:
+        if fnmatch.fnmatchcase(path, pattern):
+            return True
+    return False
+
 def get_files(storage, ignore_patterns=[], location=''):
     """
-    Recursively walk the storage directories gathering a complete
+    Recursively walk the storage directories yielding the paths
-    list of files that should be copied, returning this list.
+    of all files that should be copied.
     """
-    def is_ignored(path):
-        """
-        Return True or False depending on whether the ``path`` should be
-        ignored (if it matches any pattern in ``ignore_patterns``).
-        """
-        for pattern in ignore_patterns:
-            if fnmatch.fnmatchcase(path, pattern):
-                return True
-        return False
-
     directories, files = storage.listdir(location)
-    static_files = [location and os.path.join(location, fn) or fn
+    for fn in files:
-                    for fn in files
+        if is_ignored(fn, ignore_patterns):
-                    if not is_ignored(fn)]
+            continue
+        if location:
+            fn = os.path.join(location, fn)
+        yield fn
     for dir in directories:
-        if is_ignored(dir):
+        if is_ignored(dir, ignore_patterns):
             continue
         if location:
             dir = os.path.join(location, dir)
-        static_files.extend(get_files(storage, ignore_patterns, dir))
+        for fn in get_files(storage, ignore_patterns, dir):
-    return static_files
+            yield fn
 
 def check_settings():
     """

docs/ref/contrib/staticfiles.txt

@@ -103,9 +103,8 @@ setting.
 
 .. note::
 
-    When using the :class:`AppDirectoriesFinder` finder, make sure your apps can
+    When using the :class:`AppDirectoriesFinder` finder, make sure your apps
-    be found by Django's app loading mechanism. Simply include a ``models``
+    can be found by staticfiles. Simply add the app to the
-    module (an empty ``models.py`` file suffices) and add the app to the
     :setting:`INSTALLED_APPS` setting of your site.
 
 Static file finders are currently considered a private interface, and this

tests/regressiontests/staticfiles_tests/tests.py

@@ -34,9 +34,6 @@ class StaticFilesTestCase(TestCase):
         self.old_debug = settings.DEBUG
         self.old_installed_apps = settings.INSTALLED_APPS
 
-        # We have to load these apps to test staticfiles.
-        load_app('regressiontests.staticfiles_tests.apps.test')
-        load_app('regressiontests.staticfiles_tests.apps.no_label')
         site_media = os.path.join(TEST_ROOT, 'project', 'site_media')
         settings.DEBUG = True
         settings.MEDIA_ROOT =  os.path.join(site_media, 'media')
@@ -53,8 +50,11 @@ class StaticFilesTestCase(TestCase):
             'django.contrib.staticfiles.finders.DefaultStorageFinder',
         )
         settings.INSTALLED_APPS = [
+            'django.contrib.admin',
             'django.contrib.staticfiles',
             'regressiontests.staticfiles_tests',
+            'regressiontests.staticfiles_tests.apps.test',
+            'regressiontests.staticfiles_tests.apps.no_label',
         ]
 
         # Clear the cached default_storage out, this is because when it first