diff --git a/django/contrib/staticfiles/management/commands/collectstatic.py b/django/contrib/staticfiles/management/commands/collectstatic.py index 4d6df282c3..60c4e7cd6d 100644 --- a/django/contrib/staticfiles/management/commands/collectstatic.py +++ b/django/contrib/staticfiles/management/commands/collectstatic.py @@ -237,15 +237,14 @@ class Command(BaseCommand): if self.storage.exists(prefixed_path): try: # When was the target file modified last time? - target_last_modified = \ - self.storage.modified_time(prefixed_path) + target_last_modified = self.storage.get_modified_time(prefixed_path) except (OSError, NotImplementedError, AttributeError): - # The storage doesn't support ``modified_time`` or failed + # The storage doesn't support get_modified_time() or failed pass else: try: # When was the source file modified last time? - source_last_modified = source_storage.modified_time(path) + source_last_modified = source_storage.get_modified_time(path) except (OSError, NotImplementedError, AttributeError): pass else: diff --git a/django/core/files/storage.py b/django/core/files/storage.py index 208884e1cf..d8423f0600 100644 --- a/django/core/files/storage.py +++ b/django/core/files/storage.py @@ -1,5 +1,6 @@ import errno import os +import warnings from datetime import datetime from django.conf import settings @@ -7,9 +8,11 @@ from django.core.exceptions import SuspiciousFileOperation from django.core.files import File, locks from django.core.files.move import file_move_safe from django.core.signals import setting_changed +from django.utils import timezone from django.utils._os import abspathu, safe_join from django.utils.crypto import get_random_string from django.utils.deconstruct import deconstructible +from django.utils.deprecation import RemovedInDjango20Warning from django.utils.encoding import filepath_to_uri, force_text from django.utils.functional import LazyObject, cached_property from django.utils.module_loading import import_string @@ -140,24 +143,101 @@ class Storage(object): def accessed_time(self, name): """ Returns the last accessed time (as datetime object) of the file - specified by name. + specified by name. Deprecated: use get_accessed_time() instead. """ + warnings.warn( + 'Storage.accessed_time() is deprecated in favor of get_accessed_time().', + RemovedInDjango20Warning, + stacklevel=2, + ) raise NotImplementedError('subclasses of Storage must provide an accessed_time() method') def created_time(self, name): """ Returns the creation time (as datetime object) of the file - specified by name. + specified by name. Deprecated: use get_created_time() instead. """ + warnings.warn( + 'Storage.created_time() is deprecated in favor of get_created_time().', + RemovedInDjango20Warning, + stacklevel=2, + ) raise NotImplementedError('subclasses of Storage must provide a created_time() method') def modified_time(self, name): """ Returns the last modified time (as datetime object) of the file - specified by name. + specified by name. Deprecated: use get_modified_time() instead. """ + warnings.warn( + 'Storage.modified_time() is deprecated in favor of get_modified_time().', + RemovedInDjango20Warning, + stacklevel=2, + ) raise NotImplementedError('subclasses of Storage must provide a modified_time() method') + def get_accessed_time(self, name): + """ + Return the last accessed time (as a datetime) of the file specified by + name. The datetime will be timezone-aware if USE_TZ=True. + """ + # At the end of the deprecation: + # raise NotImplementedError('subclasses of Storage must provide a get_accessed_time() method') + warnings.warn( + 'Storage.accessed_time() is deprecated. ' + 'Storage backends should implement get_accessed_time().', + RemovedInDjango20Warning, + stacklevel=2, + ) + dt = self.accessed_time(name) + return _possibly_make_aware(dt) + + def get_created_time(self, name): + """ + Return the creation time (as a datetime) of the file specified by name. + The datetime will be timezone-aware if USE_TZ=True. + """ + # At the end of the deprecation: + # raise NotImplementedError('subclasses of Storage must provide a get_created_time() method') + warnings.warn( + 'Storage.created_time() is deprecated. ' + 'Storage backends should implement get_created_time().', + RemovedInDjango20Warning, + stacklevel=2, + ) + dt = self.created_time(name) + return _possibly_make_aware(dt) + + def get_modified_time(self, name): + """ + Return the last modified time (as a datetime) of the file specified by + name. The datetime will be timezone-aware if USE_TZ=True. + """ + # At the end of the deprecation: + # raise NotImplementedError('subclasses of Storage must provide a get_modified_time() method') + warnings.warn( + 'Storage.modified_time() is deprecated. ' + 'Storage backends should implement get_modified_time().', + RemovedInDjango20Warning, + stacklevel=2, + ) + dt = self.modified_time(name) + return _possibly_make_aware(dt) + + +def _possibly_make_aware(dt): + """ + Convert a datetime object in the local timezone to aware + in UTC, if USE_TZ is True. + """ + # This function is only needed to help with the deprecations above and can + # be removed in Django 2.0, RemovedInDjango20Warning. + if settings.USE_TZ: + tz = timezone.get_default_timezone() + return timezone.make_aware(dt, tz).astimezone(timezone.utc) + else: + return dt + @deconstructible class FileSystemStorage(Storage): @@ -328,14 +408,52 @@ class FileSystemStorage(Storage): return urljoin(self.base_url, filepath_to_uri(name)) def accessed_time(self, name): + warnings.warn( + 'FileSystemStorage.accessed_time() is deprecated in favor of ' + 'get_accessed_time().', + RemovedInDjango20Warning, + stacklevel=2, + ) return datetime.fromtimestamp(os.path.getatime(self.path(name))) def created_time(self, name): + warnings.warn( + 'FileSystemStorage.created_time() is deprecated in favor of ' + 'get_created_time().', + RemovedInDjango20Warning, + stacklevel=2, + ) return datetime.fromtimestamp(os.path.getctime(self.path(name))) def modified_time(self, name): + warnings.warn( + 'FileSystemStorage.modified_time() is deprecated in favor of ' + 'get_modified_time().', + RemovedInDjango20Warning, + stacklevel=2, + ) return datetime.fromtimestamp(os.path.getmtime(self.path(name))) + def _datetime_from_timestamp(self, ts): + """ + If timezone support is enabled, make an aware datetime object in UTC; + otherwise make a naive one in the local timezone. + """ + if settings.USE_TZ: + # Safe to use .replace() because UTC doesn't have DST + return datetime.utcfromtimestamp(ts).replace(tzinfo=timezone.utc) + else: + return datetime.fromtimestamp(ts) + + def get_accessed_time(self, name): + return self._datetime_from_timestamp(os.path.getatime(self.path(name))) + + def get_created_time(self, name): + return self._datetime_from_timestamp(os.path.getctime(self.path(name))) + + def get_modified_time(self, name): + return self._datetime_from_timestamp(os.path.getmtime(self.path(name))) + def get_storage_class(import_path=None): return import_string(import_path or settings.DEFAULT_FILE_STORAGE) diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index d1f877aa06..6e593c787d 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -135,6 +135,9 @@ details on these changes. * Support for the template ``Context.has_key()`` method will be removed. +* Support for the ``django.core.files.storage.Storage.accessed_time()``, + ``created_time()``, and ``modified_time()`` methods will be removed. + .. _deprecation-removed-in-1.10: 1.10 diff --git a/docs/ref/files/storage.txt b/docs/ref/files/storage.txt index 71354829c7..be197f7210 100644 --- a/docs/ref/files/storage.txt +++ b/docs/ref/files/storage.txt @@ -73,10 +73,9 @@ The ``Storage`` class as necessary. .. note:: - For methods returning naive ``datetime`` objects, the - effective timezone used will be the current value of - ``os.environ['TZ']``; note that this is usually set from - Django's :setting:`TIME_ZONE`. + When methods return naive ``datetime`` objects, the effective timezone + used will be the current value of ``os.environ['TZ']``; note that this + is usually set from Django's :setting:`TIME_ZONE`. .. method:: accessed_time(name) @@ -85,6 +84,10 @@ The ``Storage`` class able to return the last accessed time this will raise ``NotImplementedError`` instead. + .. deprecated:: 1.10 + + Use :meth:`get_accessed_time` instead. + .. method:: created_time(name) Returns a naive ``datetime`` object containing the creation @@ -92,6 +95,10 @@ The ``Storage`` class return the creation time this will raise ``NotImplementedError`` instead. + .. deprecated:: 1.10 + + Use :meth:`get_created_time` instead. + .. method:: delete(name) Deletes the file referenced by ``name``. If deletion is not supported @@ -104,6 +111,17 @@ The ``Storage`` class in the storage system, or ``False`` if the name is available for a new file. + .. method:: get_accessed_time(name) + + .. versionadded:: 1.10 + + Returns a :class:`~datetime.datetime` of the last accessed time of the + file. For storage systems unable to return the last accessed time this + will raise :exc:`NotImplementedError`. + + If :setting:`USE_TZ` is ``True``, returns an aware ``datetime``, + otherwise returns a naive ``datetime`` in the local timezone. + .. method:: get_available_name(name, max_length=None) Returns a filename based on the ``name`` parameter that's free and @@ -119,6 +137,28 @@ The ``Storage`` class 7 character alphanumeric string is appended to the filename before the extension. + .. method:: get_created_time(name) + + .. versionadded:: 1.10 + + Returns a :class:`~datetime.datetime` of the creation time of the file. + For storage systems unable to return the creation time this will raise + :exc:`NotImplementedError`. + + If :setting:`USE_TZ` is ``True``, returns an aware ``datetime``, + otherwise returns a naive ``datetime`` in the local timezone. + + .. method:: get_modified_time(name) + + .. versionadded:: 1.10 + + Returns a :class:`~datetime.datetime` of the last modified time of the + file. For storage systems unable to return the last modified time this + will raise :exc:`NotImplementedError`. + + If :setting:`USE_TZ` is ``True``, returns an aware ``datetime``, + otherwise returns a naive ``datetime`` in the local timezone. + .. method:: get_valid_name(name) Returns a filename based on the ``name`` parameter that's suitable @@ -138,6 +178,10 @@ The ``Storage`` class the last modified time, this will raise ``NotImplementedError`` instead. + .. deprecated:: 1.10 + + Use :meth:`get_modified_time` instead. + .. method:: open(name, mode='rb') Opens the file given by ``name``. Note that although the returned file diff --git a/docs/releases/1.10.txt b/docs/releases/1.10.txt index 8bff4e99c6..d4428ebfda 100644 --- a/docs/releases/1.10.txt +++ b/docs/releases/1.10.txt @@ -203,7 +203,13 @@ Email File Storage ~~~~~~~~~~~~ -* ... +* Storage backends now present a timezone-aware API with new methods + :meth:`~django.core.files.storage.Storage.get_accessed_time`, + :meth:`~django.core.files.storage.Storage.get_created_time`, and + :meth:`~django.core.files.storage.Storage.get_modified_time`. They return a + timezone-aware ``datetime`` if :setting:`USE_TZ` is ``True`` and a naive + ``datetime`` in the local timezone otherwise. + File Uploads ~~~~~~~~~~~~ @@ -623,6 +629,21 @@ added in Django 1.9:: This prevents confusion about an assignment resulting in an implicit save. +Non-timezone-aware :class:`~django.core.files.storage.Storage` API +------------------------------------------------------------------ + +The old, non-timezone-aware methods ``accessed_time()``, ``created_time()``, +and ``modified_time()`` are deprecated in favor of the new ``get_*_time()`` +methods. + +Third-party storage backends should implement the new methods and mark the old +ones as deprecated. Until then, the new ``get_*_time()`` methods on the base +:class:`~django.core.files.storage.Storage` class convert ``datetime``\s from +the old methods as required and emit a deprecation warning as they do so. + +Third-party storage backends may retain the old methods as long as they +wish to support earlier versions of Django. + :mod:`django.contrib.gis` ------------------------- diff --git a/tests/file_storage/tests.py b/tests/file_storage/tests.py index b5ff073525..5583b2df9c 100644 --- a/tests/file_storage/tests.py +++ b/tests/file_storage/tests.py @@ -20,14 +20,23 @@ from django.core.files.uploadedfile import ( ) from django.db.models.fields.files import FileDescriptor from django.test import ( - LiveServerTestCase, SimpleTestCase, TestCase, override_settings, + LiveServerTestCase, SimpleTestCase, TestCase, ignore_warnings, + override_settings, ) -from django.utils import six +from django.test.utils import requires_tz_support +from django.utils import six, timezone from django.utils._os import upath +from django.utils.deprecation import RemovedInDjango20Warning from django.utils.six.moves.urllib.request import urlopen from .models import Storage, temp_storage, temp_storage_location +try: + import pytz +except ImportError: + pytz = None + + FILE_SUFFIX_REGEX = '[A-Za-z0-9]{7}' @@ -83,7 +92,13 @@ class FileStorageDeconstructionTests(unittest.TestCase): self.assertEqual(kwargs, kwargs_orig) -class FileStorageTests(SimpleTestCase): +# Tests for TZ-aware time methods need pytz. +requires_pytz = unittest.skipIf(pytz is None, "this test requires pytz") + + +class FileStorageTests(TestCase): + # Changing TIME_ZONE may issue a query to set the database's timezone, + # hence TestCase. storage_class = FileSystemStorage def setUp(self): @@ -123,7 +138,86 @@ class FileStorageTests(SimpleTestCase): self.storage.delete('storage_test') self.assertFalse(self.storage.exists('storage_test')) - def test_file_accessed_time(self): + def _test_file_time_getter(self, getter): + # Check for correct behavior under both USE_TZ=True and USE_TZ=False. + # The tests are similar since they both set up a situation where the + # system time zone, Django's TIME_ZONE, and UTC are distinct. + self._test_file_time_getter_tz_handling_on(getter) + self._test_file_time_getter_tz_handling_off(getter) + + @override_settings(USE_TZ=True, TIME_ZONE='Africa/Algiers') + def _test_file_time_getter_tz_handling_on(self, getter): + # Django's TZ (and hence the system TZ) is set to Africa/Algiers which + # is UTC+1 and has no DST change. We can set the Django TZ to something + # else so that UTC, Django's TIME_ZONE, and the system timezone are all + # different. + now_in_algiers = timezone.make_aware(datetime.now()) + + # Use a fixed offset timezone so we don't need pytz. + with timezone.override(timezone.get_fixed_timezone(-300)): + # At this point the system TZ is +1 and the Django TZ + # is -5. The following will be aware in UTC. + now = timezone.now() + self.assertFalse(self.storage.exists('test.file.tz.on')) + + f = ContentFile('custom contents') + f_name = self.storage.save('test.file.tz.on', f) + self.addCleanup(self.storage.delete, f_name) + dt = getter(f_name) + # dt should be aware, in UTC + self.assertTrue(timezone.is_aware(dt)) + self.assertEqual(now.tzname(), dt.tzname()) + + # Check that the three timezones are indeed distinct. + naive_now = datetime.now() + algiers_offset = now_in_algiers.tzinfo.utcoffset(naive_now) + django_offset = timezone.get_current_timezone().utcoffset(naive_now) + utc_offset = timezone.utc.utcoffset(naive_now) + self.assertGreater(algiers_offset, utc_offset) + self.assertLess(django_offset, utc_offset) + + # dt and now should be the same effective time. + self.assertLess(abs(dt - now), timedelta(seconds=2)) + + @override_settings(USE_TZ=False, TIME_ZONE='Africa/Algiers') + def _test_file_time_getter_tz_handling_off(self, getter): + # Django's TZ (and hence the system TZ) is set to Africa/Algiers which + # is UTC+1 and has no DST change. We can set the Django TZ to something + # else so that UTC, Django's TIME_ZONE, and the system timezone are all + # different. + now_in_algiers = timezone.make_aware(datetime.now()) + + # Use a fixed offset timezone so we don't need pytz. + with timezone.override(timezone.get_fixed_timezone(-300)): + # At this point the system TZ is +1 and the Django TZ + # is -5. + self.assertFalse(self.storage.exists('test.file.tz.off')) + + f = ContentFile('custom contents') + f_name = self.storage.save('test.file.tz.off', f) + self.addCleanup(self.storage.delete, f_name) + dt = getter(f_name) + # dt should be naive, in system (+1) TZ + self.assertTrue(timezone.is_naive(dt)) + + # Check that the three timezones are indeed distinct. + naive_now = datetime.now() + algiers_offset = now_in_algiers.tzinfo.utcoffset(naive_now) + django_offset = timezone.get_current_timezone().utcoffset(naive_now) + utc_offset = timezone.utc.utcoffset(naive_now) + self.assertGreater(algiers_offset, utc_offset) + self.assertLess(django_offset, utc_offset) + + # dt and naive_now should be the same effective time. + self.assertLess(abs(dt - naive_now), timedelta(seconds=2)) + # If we convert dt to an aware object using the Algiers + # timezone then it should be the same effective time to + # now_in_algiers. + _dt = timezone.make_aware(dt, now_in_algiers.tzinfo) + self.assertLess(abs(_dt - now_in_algiers), timedelta(seconds=2)) + + @requires_pytz + def test_file_get_accessed_time(self): """ File storage returns a Datetime object for the last accessed time of a file. @@ -132,47 +226,102 @@ class FileStorageTests(SimpleTestCase): f = ContentFile('custom contents') f_name = self.storage.save('test.file', f) + self.addCleanup(self.storage.delete, f_name) + atime = self.storage.get_accessed_time(f_name) + + self.assertEqual(atime, datetime.fromtimestamp(os.path.getatime(self.storage.path(f_name)))) + self.assertLess(timezone.now() - self.storage.get_accessed_time(f_name), timedelta(seconds=2)) + + @requires_pytz + @requires_tz_support + def test_file_get_accessed_time_timezone(self): + self._test_file_time_getter(self.storage.get_accessed_time) + + @ignore_warnings(category=RemovedInDjango20Warning) + def test_file_accessed_time(self): + """ + File storage returns a datetime for the last accessed time of a file. + """ + self.assertFalse(self.storage.exists('test.file')) + + f = ContentFile('custom contents') + f_name = self.storage.save('test.file', f) + self.addCleanup(self.storage.delete, f_name) atime = self.storage.accessed_time(f_name) - self.assertEqual(atime, datetime.fromtimestamp( - os.path.getatime(self.storage.path(f_name)))) + self.assertEqual(atime, datetime.fromtimestamp(os.path.getatime(self.storage.path(f_name)))) self.assertLess(datetime.now() - self.storage.accessed_time(f_name), timedelta(seconds=2)) - self.storage.delete(f_name) + @requires_pytz + def test_file_get_created_time(self): + """ + File storage returns a datetime for the creation time of a file. + """ + self.assertFalse(self.storage.exists('test.file')) + + f = ContentFile('custom contents') + f_name = self.storage.save('test.file', f) + self.addCleanup(self.storage.delete, f_name) + ctime = self.storage.get_created_time(f_name) + + self.assertEqual(ctime, datetime.fromtimestamp(os.path.getctime(self.storage.path(f_name)))) + self.assertLess(timezone.now() - self.storage.get_created_time(f_name), timedelta(seconds=2)) + + @requires_pytz + @requires_tz_support + def test_file_get_created_time_timezone(self): + self._test_file_time_getter(self.storage.get_created_time) + + @ignore_warnings(category=RemovedInDjango20Warning) def test_file_created_time(self): """ - File storage returns a Datetime object for the creation time of - a file. + File storage returns a datetime for the creation time of a file. """ self.assertFalse(self.storage.exists('test.file')) f = ContentFile('custom contents') f_name = self.storage.save('test.file', f) ctime = self.storage.created_time(f_name) + self.addCleanup(self.storage.delete, f_name) - self.assertEqual(ctime, datetime.fromtimestamp( - os.path.getctime(self.storage.path(f_name)))) + self.assertEqual(ctime, datetime.fromtimestamp(os.path.getctime(self.storage.path(f_name)))) self.assertLess(datetime.now() - self.storage.created_time(f_name), timedelta(seconds=2)) - self.storage.delete(f_name) - - def test_file_modified_time(self): + @requires_pytz + def test_file_get_modified_time(self): """ - File storage returns a Datetime object for the last modified time of - a file. + File storage returns a datetime for the last modified time of a file. """ self.assertFalse(self.storage.exists('test.file')) f = ContentFile('custom contents') f_name = self.storage.save('test.file', f) + self.addCleanup(self.storage.delete, f_name) + mtime = self.storage.get_modified_time(f_name) + + self.assertEqual(mtime, datetime.fromtimestamp(os.path.getmtime(self.storage.path(f_name)))) + self.assertLess(timezone.now() - self.storage.get_modified_time(f_name), timedelta(seconds=2)) + + @requires_pytz + @requires_tz_support + def test_file_get_modified_time_timezone(self): + self._test_file_time_getter(self.storage.get_modified_time) + + @ignore_warnings(category=RemovedInDjango20Warning) + def test_file_modified_time(self): + """ + File storage returns a datetime for the last modified time of a file. + """ + self.assertFalse(self.storage.exists('test.file')) + + f = ContentFile('custom contents') + f_name = self.storage.save('test.file', f) + self.addCleanup(self.storage.delete, f_name) mtime = self.storage.modified_time(f_name) - self.assertEqual(mtime, datetime.fromtimestamp( - os.path.getmtime(self.storage.path(f_name)))) + self.assertEqual(mtime, datetime.fromtimestamp(os.path.getmtime(self.storage.path(f_name)))) self.assertLess(datetime.now() - self.storage.modified_time(f_name), timedelta(seconds=2)) - self.storage.delete(f_name) - def test_file_save_without_name(self): """ File storage extracts the filename from the content object if no @@ -473,6 +622,26 @@ class CustomStorageTests(FileStorageTests): self.storage.delete(second) +class CustomStorageLegacyDatetimeHandling(FileSystemStorage): + # Use the legacy accessed_time() et al from FileSystemStorage and the + # shim get_accessed_time() et al from the Storage baseclass. Both of those + # raise warnings, so the testcase class ignores them all. + + def get_accessed_time(self, name): + return super(FileSystemStorage, self).get_accessed_time(name) + + def get_created_time(self, name): + return super(FileSystemStorage, self).get_created_time(name) + + def get_modified_time(self, name): + return super(FileSystemStorage, self).get_modified_time(name) + + +@ignore_warnings(category=RemovedInDjango20Warning) +class CustomStorageLegacyDatetimeHandlingTests(FileStorageTests): + storage_class = CustomStorageLegacyDatetimeHandling + + class FileFieldStorageTests(TestCase): def tearDown(self): shutil.rmtree(temp_storage_location) diff --git a/tests/staticfiles_tests/storage.py b/tests/staticfiles_tests/storage.py index fe4811a495..3e972552f2 100644 --- a/tests/staticfiles_tests/storage.py +++ b/tests/staticfiles_tests/storage.py @@ -2,12 +2,12 @@ from datetime import datetime from django.contrib.staticfiles.storage import CachedStaticFilesStorage from django.core.files import storage +from django.utils import timezone class DummyStorage(storage.Storage): """ - A storage class that does implement modified_time() but raises - NotImplementedError when calling + A storage class that implements get_modified_time(). """ def _save(self, name, content): return 'dummy' @@ -18,8 +18,8 @@ class DummyStorage(storage.Storage): def exists(self, name): pass - def modified_time(self, name): - return datetime.date(1970, 1, 1) + def get_modified_time(self, name): + return datetime.datetime(1970, 1, 1, tzinfo=timezone.utc) class SimpleCachedStaticFilesStorage(CachedStaticFilesStorage):