diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 968719759eb..02ac01cd354 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -1079,6 +1079,7 @@ class DateTimeCheckMixin(object): def check(self, **kwargs): errors = super(DateTimeCheckMixin, self).check(**kwargs) errors.extend(self._check_mutually_exclusive_options()) + errors.extend(self._check_fix_default_value()) return errors def _check_mutually_exclusive_options(self): @@ -1103,6 +1104,9 @@ class DateTimeCheckMixin(object): else: return [] + def _check_fix_default_value(self): + return [] + class DateField(DateTimeCheckMixin, Field): empty_strings_allowed = False @@ -1122,6 +1126,49 @@ class DateField(DateTimeCheckMixin, Field): kwargs['blank'] = True super(DateField, self).__init__(verbose_name, name, **kwargs) + def _check_fix_default_value(self): + """ + Adds a warning to the checks framework stating, that using an actual + date or datetime value is probably wrong; it's only being evaluated on + server start-up. + + For details see ticket #21905 + """ + if not self.has_default(): + return [] + + now = timezone.now() + if not timezone.is_naive(now): + now = timezone.make_naive(now, timezone.utc) + value = self.default + if isinstance(value, datetime.datetime): + if not timezone.is_naive(value): + value = timezone.make_naive(value, timezone.utc) + value = value.date() + elif isinstance(value, datetime.date): + # Nothing to do, as dates don't have tz information + pass + else: + # No explicit date / datetime value -- no checks necessary + return [] + offset = datetime.timedelta(days=1) + lower = (now - offset).date() + upper = (now + offset).date() + if lower <= value <= upper: + return [ + checks.Warning( + 'Fixed default value provided.', + hint='It seems you set a fixed date / time / datetime ' + 'value as default for this field. This may not be ' + 'what you want. If you want to have the current date ' + 'as default, use `django.utils.timezone.now`', + obj=self, + id='fields.W161', + ) + ] + + return [] + def deconstruct(self): name, path, args, kwargs = super(DateField, self).deconstruct() if self.auto_now: @@ -1226,6 +1273,52 @@ class DateTimeField(DateField): # __init__ is inherited from DateField + def _check_fix_default_value(self): + """ + Adds a warning to the checks framework stating, that using an actual + date or datetime value is probably wrong; it's only being evaluated on + server start-up. + + For details see ticket #21905 + """ + if not self.has_default(): + return [] + + now = timezone.now() + if not timezone.is_naive(now): + now = timezone.make_naive(now, timezone.utc) + value = self.default + if isinstance(value, datetime.datetime): + second_offset = datetime.timedelta(seconds=10) + lower = now - second_offset + upper = now + second_offset + if timezone.is_aware(value): + value = timezone.make_naive(value, timezone.utc) + elif isinstance(value, datetime.date): + second_offset = datetime.timedelta(seconds=10) + lower = now - second_offset + lower = datetime.datetime(lower.year, lower.month, lower.day) + upper = now + second_offset + upper = datetime.datetime(upper.year, upper.month, upper.day) + value = datetime.datetime(value.year, value.month, value.day) + else: + # No explicit date / datetime value -- no checks necessary + return [] + if lower <= value <= upper: + return [ + checks.Warning( + 'Fixed default value provided.', + hint='It seems you set a fixed date / time / datetime ' + 'value as default for this field. This may not be ' + 'what you want. If you want to have the current date ' + 'as default, use `django.utils.timezone.now`', + obj=self, + id='fields.W161', + ) + ] + + return [] + def get_internal_type(self): return "DateTimeField" @@ -1935,6 +2028,52 @@ class TimeField(DateTimeCheckMixin, Field): kwargs['blank'] = True super(TimeField, self).__init__(verbose_name, name, **kwargs) + def _check_fix_default_value(self): + """ + Adds a warning to the checks framework stating, that using an actual + time or datetime value is probably wrong; it's only being evaluated on + server start-up. + + For details see ticket #21905 + """ + if not self.has_default(): + return [] + + now = timezone.now() + if not timezone.is_naive(now): + now = timezone.make_naive(now, timezone.utc) + value = self.default + if isinstance(value, datetime.datetime): + second_offset = datetime.timedelta(seconds=10) + lower = now - second_offset + upper = now + second_offset + if timezone.is_aware(value): + value = timezone.make_naive(value, timezone.utc) + elif isinstance(value, datetime.time): + second_offset = datetime.timedelta(seconds=10) + lower = now - second_offset + upper = now + second_offset + value = datetime.datetime.combine(now.date(), value) + if timezone.is_aware(value): + value = timezone.make_naive(value, timezone.utc).time() + else: + # No explicit time / datetime value -- no checks necessary + return [] + if lower <= value <= upper: + return [ + checks.Warning( + 'Fixed default value provided.', + hint='It seems you set a fixed date / time / datetime ' + 'value as default for this field. This may not be ' + 'what you want. If you want to have the current date ' + 'as default, use `django.utils.timezone.now`', + obj=self, + id='fields.W161', + ) + ] + + return [] + def deconstruct(self): name, path, args, kwargs = super(TimeField, self).deconstruct() if self.auto_now is not False: diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 0787bd66357..8cdeb1df91e 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -68,6 +68,8 @@ Fields * **fields.E140**: FilePathFields must have either ``allow_files`` or ``allow_folders`` set to True. * **fields.E150**: GenericIPAddressFields cannot accept blank values if null values are not allowed, as blank values are stored as nulls. * **fields.E160**: The options ``auto_now``, ``auto_now_add``, and ``default`` are mutually exclusive. Only one of these options may be present. +* **fields.W161**: Fixed default value provided. + File Fields ~~~~~~~~~~~ diff --git a/tests/invalid_models_tests/test_ordinary_fields.py b/tests/invalid_models_tests/test_ordinary_fields.py index 1d5ed6ea856..176a932c499 100644 --- a/tests/invalid_models_tests/test_ordinary_fields.py +++ b/tests/invalid_models_tests/test_ordinary_fields.py @@ -1,11 +1,12 @@ # -*- encoding: utf-8 -*- from __future__ import unicode_literals -from datetime import datetime import unittest -from django.core.checks import Error +from django.core.checks import Error, Warning as DjangoWarning from django.db import connection, models +from django.test.utils import override_settings +from django.utils.timezone import make_aware, now from .base import IsolatedModelsTestCase @@ -198,6 +199,116 @@ class CharFieldTests(IsolatedModelsTestCase): self.assertEqual(errors, expected) +class DateFieldTests(IsolatedModelsTestCase): + + def test_auto_now_and_auto_now_add_raise_error(self): + class Model(models.Model): + field0 = models.DateTimeField(auto_now=True, auto_now_add=True, default=now) + field1 = models.DateTimeField(auto_now=True, auto_now_add=False, default=now) + field2 = models.DateTimeField(auto_now=False, auto_now_add=True, default=now) + field3 = models.DateTimeField(auto_now=True, auto_now_add=True, default=None) + + expected = [] + checks = [] + for i in range(4): + field = Model._meta.get_field('field%d' % i) + expected.append(Error( + "The options auto_now, auto_now_add, and default " + "are mutually exclusive. Only one of these options " + "may be present.", + hint=None, + obj=field, + id='fields.E160', + )) + checks.extend(field.check()) + self.assertEqual(checks, expected) + + def test_fix_default_value(self): + class Model(models.Model): + field_dt = models.DateField(default=now()) + field_d = models.DateField(default=now().date()) + field_now = models.DateField(default=now) + + field_dt = Model._meta.get_field('field_dt') + field_d = Model._meta.get_field('field_d') + field_now = Model._meta.get_field('field_now') + errors = field_dt.check() + errors.extend(field_d.check()) + errors.extend(field_now.check()) # doesn't raise a warning + expected = [ + DjangoWarning( + 'Fixed default value provided.', + hint='It seems you set a fixed date / time / datetime ' + 'value as default for this field. This may not be ' + 'what you want. If you want to have the current date ' + 'as default, use `django.utils.timezone.now`', + obj=field_dt, + id='fields.W161', + ), + DjangoWarning( + 'Fixed default value provided.', + hint='It seems you set a fixed date / time / datetime ' + 'value as default for this field. This may not be ' + 'what you want. If you want to have the current date ' + 'as default, use `django.utils.timezone.now`', + obj=field_d, + id='fields.W161', + ) + ] + maxDiff = self.maxDiff + self.maxDiff = None + self.assertEqual(errors, expected) + self.maxDiff = maxDiff + + @override_settings(USE_TZ=True) + def test_fix_default_value_tz(self): + self.test_fix_default_value() + + +class DateTimeFieldTests(IsolatedModelsTestCase): + + def test_fix_default_value(self): + class Model(models.Model): + field_dt = models.DateTimeField(default=now()) + field_d = models.DateTimeField(default=now().date()) + field_now = models.DateTimeField(default=now) + + field_dt = Model._meta.get_field('field_dt') + field_d = Model._meta.get_field('field_d') + field_now = Model._meta.get_field('field_now') + errors = field_dt.check() + errors.extend(field_d.check()) + errors.extend(field_now.check()) # doesn't raise a warning + expected = [ + DjangoWarning( + 'Fixed default value provided.', + hint='It seems you set a fixed date / time / datetime ' + 'value as default for this field. This may not be ' + 'what you want. If you want to have the current date ' + 'as default, use `django.utils.timezone.now`', + obj=field_dt, + id='fields.W161', + ), + DjangoWarning( + 'Fixed default value provided.', + hint='It seems you set a fixed date / time / datetime ' + 'value as default for this field. This may not be ' + 'what you want. If you want to have the current date ' + 'as default, use `django.utils.timezone.now`', + obj=field_d, + id='fields.W161', + ) + ] + maxDiff = self.maxDiff + self.maxDiff = None + self.assertEqual(errors, expected) + self.maxDiff = maxDiff + + @override_settings(USE_TZ=True) + def test_fix_default_value_tz(self): + self.test_fix_default_value() + + class DecimalFieldTests(IsolatedModelsTestCase): def test_required_attributes(self): @@ -402,28 +513,45 @@ class ImageFieldTests(IsolatedModelsTestCase): self.assertEqual(errors, expected) -class DateFieldTests(IsolatedModelsTestCase): +class TimeFieldTests(IsolatedModelsTestCase): - def test_auto_now_and_auto_now_add_raise_error(self): - dn = datetime.now - mutually_exclusive_combinations = ( - (True, True, dn), - (True, False, dn), - (False, True, dn), - (True, True, None) + def test_fix_default_value(self): + class Model(models.Model): + field_dt = models.TimeField(default=now()) + field_t = models.TimeField(default=now().time()) + field_now = models.DateField(default=now) + + field_dt = Model._meta.get_field('field_dt') + field_t = Model._meta.get_field('field_t') + field_now = Model._meta.get_field('field_now') + errors = field_dt.check() + errors.extend(field_t.check()) + errors.extend(field_now.check()) # doesn't raise a warning + expected = [ + DjangoWarning( + 'Fixed default value provided.', + hint='It seems you set a fixed date / time / datetime ' + 'value as default for this field. This may not be ' + 'what you want. If you want to have the current date ' + 'as default, use `django.utils.timezone.now`', + obj=field_dt, + id='fields.W161', + ), + DjangoWarning( + 'Fixed default value provided.', + hint='It seems you set a fixed date / time / datetime ' + 'value as default for this field. This may not be ' + 'what you want. If you want to have the current date ' + 'as default, use `django.utils.timezone.now`', + obj=field_t, + id='fields.W161', ) + ] + maxDiff = self.maxDiff + self.maxDiff = None + self.assertEqual(errors, expected) + self.maxDiff = maxDiff - for auto_now, auto_now_add, default in mutually_exclusive_combinations: - field = models.DateTimeField(name="field", auto_now=auto_now, - auto_now_add=auto_now_add, - default=default) - expected = [Error( - "The options auto_now, auto_now_add, and default " - "are mutually exclusive. Only one of these options " - "may be present.", - hint=None, - obj=field, - id='fields.E160', - )] - checks = field.check() - self.assertEqual(checks, expected) + @override_settings(USE_TZ=True) + def test_fix_default_value_tz(self): + self.test_fix_default_value()