Fixed #15035 -- Fixed collectstatic management command to work with non-local storage backends correctly. Also refactored a bit code smell.

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

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

@@ -32,23 +32,27 @@ class Command(NoArgsCommand):
     )
     help = "Collect static files from apps and other locations in a single location."
 
-    def handle_noargs(self, **options):
+    def __init__(self, *args, **kwargs):
-        symlink = options['link']
-        ignore_patterns = options['ignore_patterns']
-        if options['use_default_ignore_patterns']:
-            ignore_patterns += ['CVS', '.*', '*~']
-        ignore_patterns = list(set(ignore_patterns))
         self.copied_files = set()
         self.symlinked_files = set()
         self.unmodified_files = set()
         self.destination_storage = get_storage_class(settings.STATICFILES_STORAGE)()
-
         try:
             self.destination_storage.path('')
         except NotImplementedError:
             self.destination_local = False
         else:
             self.destination_local = True
+        # Use ints for file times (ticket #14665)
+        os.stat_float_times(False)
+
+    def handle_noargs(self, **options):
+        symlink = options['link']
+        ignore_patterns = options['ignore_patterns']
+        if options['use_default_ignore_patterns']:
+            ignore_patterns += ['CVS', '.*', '*~']
+        ignore_patterns = list(set(ignore_patterns))
+        self.verbosity = int(options.get('verbosity', 1))
 
         if symlink:
             if sys.platform == 'win32':
@@ -70,17 +74,13 @@ Type 'yes' to continue, or 'no' to cancel: """)
             if confirm != 'yes':
                 raise CommandError("Collecting static files cancelled.")
 
-        # Use ints for file times (ticket #14665)
-        os.stat_float_times(False)
-
         for finder in finders.get_finders():
             for source, prefix, storage in finder.list(ignore_patterns):
                 self.copy_file(source, prefix, storage, **options)
 
-        verbosity = int(options.get('verbosity', 1))
         actual_count = len(self.copied_files) + len(self.symlinked_files)
         unmodified_count = len(self.unmodified_files)
-        if verbosity >= 1:
+        if self.verbosity >= 1:
             self.stdout.write("\n%s static file%s %s to '%s'%s.\n"
                               % (actual_count, actual_count != 1 and 's' or '',
                                  symlink and 'symlinked' or 'copied',
@@ -88,6 +88,15 @@ Type 'yes' to continue, or 'no' to cancel: """)
                                  unmodified_count and ' (%s unmodified)'
                                  % unmodified_count or ''))
 
+    def log(self, msg, level=2):
+        """
+        Small log helper
+        """
+        if not msg.endswith("\n"):
+            msg += "\n"
+        if self.verbosity >= level:
+            self.stdout.write(msg)
+
     def copy_file(self, source, prefix, source_storage, **options):
         """
         Attempt to copy (or symlink) ``source`` to ``destination``,
@@ -104,18 +113,13 @@ Type 'yes' to continue, or 'no' to cancel: """)
             destination = source
         symlink = options['link']
         dry_run = options['dry_run']
-        verbosity = int(options.get('verbosity', 1))
 
         if destination in self.copied_files:
-            if verbosity >= 2:
+            self.log("Skipping '%s' (already copied earlier)" % destination)
-                self.stdout.write("Skipping '%s' (already copied earlier)\n"
-                                  % destination)
             return False
 
         if destination in self.symlinked_files:
-            if verbosity >= 2:
+            self.log("Skipping '%s' (already linked earlier)" % destination)
-                self.stdout.write("Skipping '%s' (already linked earlier)\n"
-                                  % destination)
             return False
 
         if self.destination_storage.exists(destination):
@@ -126,34 +130,27 @@ Type 'yes' to continue, or 'no' to cancel: """)
                 # storage doesn't support ``modified_time`` or failed.
                 pass
             else:
-                destination_is_link = os.path.islink(
+                destination_is_link = (self.destination_local and
-                    self.destination_storage.path(destination))
+                    os.path.islink(self.destination_storage.path(destination)))
                 if destination_last_modified >= source_last_modified:
                     if (not symlink and not destination_is_link):
-                        if verbosity >= 2:
+                        self.log("Skipping '%s' (not modified)" % destination)
-                            self.stdout.write("Skipping '%s' (not modified)\n"
-                                              % destination)
                         self.unmodified_files.add(destination)
                         return False
             if dry_run:
-                if verbosity >= 2:
+                self.log("Pretending to delete '%s'" % destination)
-                    self.stdout.write("Pretending to delete '%s'\n"
-                                      % destination)
             else:
-                if verbosity >= 2:
+                self.log("Deleting '%s'" % destination)
-                    self.stdout.write("Deleting '%s'\n" % destination)
                 self.destination_storage.delete(destination)
 
         if symlink:
             destination_path = self.destination_storage.path(destination)
             if dry_run:
-                if verbosity >= 1:
+                self.log("Pretending to link '%s' to '%s'" %
-                    self.stdout.write("Pretending to symlink '%s' to '%s'\n"
+                         (source_path, destination_path), level=1)
-                                      % (source_path, destination_path))
             else:
-                if verbosity >= 1:
+                self.log("Linking '%s' to '%s'" %
-                    self.stdout.write("Symlinking '%s' to '%s'\n"
+                         (source_path, destination_path), level=1)
-                                      % (source_path, destination_path))
                 try:
                     os.makedirs(os.path.dirname(destination_path))
                 except OSError:
@@ -162,9 +159,8 @@ Type 'yes' to continue, or 'no' to cancel: """)
             self.symlinked_files.add(destination)
         else:
             if dry_run:
-                if verbosity >= 1:
+                self.log("Pretending to copy '%s' to '%s'" %
-                    self.stdout.write("Pretending to copy '%s' to '%s'\n"
+                         (source_path, destination), level=1)
-                                      % (source_path, destination))
             else:
                 if self.destination_local:
                     destination_path = self.destination_storage.path(destination)
@@ -173,14 +169,12 @@ Type 'yes' to continue, or 'no' to cancel: """)
                     except OSError:
                         pass
                     shutil.copy2(source_path, destination_path)
-                    if verbosity >= 1:
+                    self.log("Copying '%s' to '%s'" %
-                        self.stdout.write("Copying '%s' to '%s'\n"
+                             (source_path, destination_path), level=1)
-                                          % (source_path, destination_path))
                 else:
                     source_file = source_storage.open(source)
                     self.destination_storage.save(destination, source_file)
-                    if verbosity >= 1:
+                    self.log("Copying %s to %s" %
-                        self.stdout.write("Copying %s to %s\n"
+                             (source_path, destination), level=1)
-                                          % (source_path, destination))
             self.copied_files.add(destination)
         return True

tests/regressiontests/staticfiles_tests/storage.py

@@ -0,0 +1,19 @@
+from datetime import datetime
+from django.core.files import storage
+
+class DummyStorage(storage.Storage):
+    """
+    A storage class that does implement modified_time() but raises
+    NotImplementedError when calling 
+    """
+    def _save(self, name, content):
+        return 'dummy'
+
+    def delete(self, name):
+        pass
+
+    def exists(self, name):
+        pass
+
+    def modified_time(self, name):
+        return datetime.date(1970, 1, 1)

tests/regressiontests/staticfiles_tests/tests.py

@@ -92,7 +92,6 @@ class BuildStaticTestCase(StaticFilesTestCase):
     """
     def setUp(self):
         super(BuildStaticTestCase, self).setUp()
-        self.old_staticfiles_storage = settings.STATICFILES_STORAGE
         self.old_root = settings.STATIC_ROOT
         settings.STATIC_ROOT = tempfile.mkdtemp()
         self.run_collectstatic()
@@ -220,18 +219,35 @@ class TestBuildStaticExcludeNoDefaultIgnore(BuildStaticTestCase, TestDefaults):
         self.assertFileContains('test/CVS', 'should be ignored')
 
 
-class TestBuildStaticDryRun(BuildStaticTestCase):
+class TestNoFilesCreated(object):
+
+    def test_no_files_created(self):
+        """
+        Make sure no files were create in the destination directory.
+        """
+        self.assertEquals(os.listdir(settings.STATIC_ROOT), [])
+
+
+class TestBuildStaticDryRun(BuildStaticTestCase, TestNoFilesCreated):
     """
     Test ``--dry-run`` option for ``collectstatic`` management command.
     """
     def run_collectstatic(self):
         super(TestBuildStaticDryRun, self).run_collectstatic(dry_run=True)
 
-    def test_no_files_created(self):
+
-        """
+class TestBuildStaticNonLocalStorage(BuildStaticTestCase, TestNoFilesCreated):
-        With --dry-run, no files created in destination dir.
+    """
-        """
+    Tests for #15035
-        self.assertEquals(os.listdir(settings.STATIC_ROOT), [])
+    """
+    def setUp(self):
+        self.old_staticfiles_storage = settings.STATICFILES_STORAGE
+        settings.STATICFILES_STORAGE = 'regressiontests.staticfiles_tests.storage.DummyStorage'
+        super(TestBuildStaticNonLocalStorage, self).setUp()
+
+    def tearDown(self):
+        super(TestBuildStaticNonLocalStorage, self).tearDown()
+        settings.STATICFILES_STORAGE = self.old_staticfiles_storage
 
 
 if sys.platform != 'win32':