From 49c57f8565d4b655de5161d299a053eddfc00bf2 Mon Sep 17 00:00:00 2001 From: Iacopo Spalletti Date: Sat, 2 Apr 2016 15:49:32 +0200 Subject: [PATCH] Fixed #25005 -- Made date and time fields with auto_now/auto_now_add use effective default. Thanks to Andriy Sokolovskiy for initial patch. --- django/db/backends/base/schema.py | 12 ++- django/db/migrations/autodetector.py | 12 ++- django/db/migrations/questioner.py | 53 ++++++++++++- .../test_auto_now_add/0001_initial.py | 19 +++++ .../migrations/test_auto_now_add/__init__.py | 0 tests/migrations/test_autodetector.py | 57 ++++++++++++++ tests/migrations/test_commands.py | 25 +++++++ tests/schema/tests.py | 74 ++++++++++++++++++- 8 files changed, 243 insertions(+), 9 deletions(-) create mode 100644 tests/migrations/test_auto_now_add/0001_initial.py create mode 100644 tests/migrations/test_auto_now_add/__init__.py diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index f91ab72e3d..2fd0d2738c 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -1,9 +1,10 @@ import hashlib import logging +from datetime import datetime from django.db.backends.utils import truncate_name from django.db.transaction import atomic -from django.utils import six +from django.utils import six, timezone from django.utils.encoding import force_bytes logger = logging.getLogger('django.db.backends.schema') @@ -201,6 +202,15 @@ class BaseDatabaseSchemaEditor(object): default = six.binary_type() else: default = six.text_type() + elif getattr(field, 'auto_now', False) or getattr(field, 'auto_now_add', False): + default = datetime.now() + internal_type = field.get_internal_type() + if internal_type == 'DateField': + default = default.date + elif internal_type == 'TimeField': + default = default.time + elif internal_type == 'DateTimeField': + default = timezone.now else: default = None # If it's a callable, call it diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index 96aa230b24..85e50746db 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -802,10 +802,16 @@ class MigrationAutodetector(object): # You can't just add NOT NULL fields with no default or fields # which don't allow empty strings as default. preserve_default = True - if (not field.null and not field.has_default() and not field.many_to_many and - not (field.blank and field.empty_strings_allowed)): + time_fields = (models.DateField, models.DateTimeField, models.TimeField) + if (not field.null and not field.has_default() and + not field.many_to_many and + not (field.blank and field.empty_strings_allowed) and + not (isinstance(field, time_fields) and field.auto_now)): field = field.clone() - field.default = self.questioner.ask_not_null_addition(field_name, model_name) + if isinstance(field, time_fields) and field.auto_now_add: + field.default = self.questioner.ask_auto_now_add_addition(field_name, model_name) + else: + field.default = self.questioner.ask_not_null_addition(field_name, model_name) preserve_default = False self.add_operation( app_label, diff --git a/django/db/migrations/questioner.py b/django/db/migrations/questioner.py index 0315df0630..36144f1a55 100644 --- a/django/db/migrations/questioner.py +++ b/django/db/migrations/questioner.py @@ -76,6 +76,11 @@ class MigrationQuestioner(object): "Do you really want to merge these migrations?" return self.defaults.get("ask_merge", False) + def ask_auto_now_add_addition(self, field_name, model_name): + "Adding an auto_now_add field to a model" + # None means quit + return None + class InteractiveMigrationQuestioner(MigrationQuestioner): @@ -101,17 +106,36 @@ class InteractiveMigrationQuestioner(MigrationQuestioner): pass result = input("Please select a valid option: ") - def _ask_default(self): + def _ask_default(self, default=''): + """ + Prompt for a default value. + + The ``default`` argument allows providing a custom default value (as a + string) which will be shown to the user and used as the return value + if the user doesn't provide any other input. + """ print("Please enter the default value now, as valid Python") + if default: + print( + "You can accept the default '{}' by pressing 'Enter' or you " + "can provide another value.".format(default) + ) print("The datetime and django.utils.timezone modules are available, so you can do e.g. timezone.now") + print("Type 'exit' to exit this prompt") while True: + if default: + prompt = "[default: {}] >>> ".format(default) + else: + prompt = ">>> " if six.PY3: # Six does not correctly abstract over the fact that # py3 input returns a unicode string, while py2 raw_input # returns a bytestring. - code = input(">>> ") + code = input(prompt) else: - code = input(">>> ").decode(sys.stdin.encoding) + code = input(prompt).decode(sys.stdin.encoding) + if not code and default: + code = default if not code: print("Please enter some code, or 'exit' (with no quotes) to exit.") elif code == "exit": @@ -186,6 +210,25 @@ class InteractiveMigrationQuestioner(MigrationQuestioner): False, ) + def ask_auto_now_add_addition(self, field_name, model_name): + "Adding an auto_now_add field to a model" + if not self.dry_run: + choice = self._choice_input( + "You are trying to add the field '{}' with 'auto_now_add=True' " + "to {} without a default; the database needs something to " + "populate existing rows.\n".format(field_name, model_name), + [ + "Provide a one-off default now (will be set on all " + "existing rows)", + "Quit, and let me add a default in models.py", + ] + ) + if choice == 2: + sys.exit(3) + else: + return self._ask_default(default='timezone.now') + return None + class NonInteractiveMigrationQuestioner(MigrationQuestioner): @@ -196,3 +239,7 @@ class NonInteractiveMigrationQuestioner(MigrationQuestioner): def ask_not_null_alteration(self, field_name, model_name): # We can't ask the user, so set as not provided. return NOT_PROVIDED + + def ask_auto_now_add_addition(self, field_name, model_name): + # We can't ask the user, so act like the user aborted. + sys.exit(3) diff --git a/tests/migrations/test_auto_now_add/0001_initial.py b/tests/migrations/test_auto_now_add/0001_initial.py new file mode 100644 index 0000000000..f966cc0e7b --- /dev/null +++ b/tests/migrations/test_auto_now_add/0001_initial.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + operations = [ + migrations.CreateModel( + name='Entry', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('title', models.CharField(max_length=255)), + ], + ), + ] diff --git a/tests/migrations/test_auto_now_add/__init__.py b/tests/migrations/test_auto_now_add/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 25b6a3190b..42c84c0ced 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -61,6 +61,18 @@ class AutodetectorTests(TestCase): ("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200, default='Ada Lovelace')), ]) + author_dates_of_birth_auto_now = ModelState("testapp", "Author", [ + ("id", models.AutoField(primary_key=True)), + ("date_of_birth", models.DateField(auto_now=True)), + ("date_time_of_birth", models.DateTimeField(auto_now=True)), + ("time_of_birth", models.TimeField(auto_now=True)), + ]) + author_dates_of_birth_auto_now_add = ModelState("testapp", "Author", [ + ("id", models.AutoField(primary_key=True)), + ("date_of_birth", models.DateField(auto_now_add=True)), + ("date_time_of_birth", models.DateTimeField(auto_now_add=True)), + ("time_of_birth", models.TimeField(auto_now_add=True)), + ]) author_name_deconstructible_1 = ModelState("testapp", "Author", [ ("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200, default=DeconstructibleObject())), @@ -634,6 +646,51 @@ class AutodetectorTests(TestCase): self.assertOperationTypes(changes, 'testapp', 0, ["AddField"]) self.assertOperationAttributes(changes, "testapp", 0, 0, name="name") + @mock.patch('django.db.migrations.questioner.MigrationQuestioner.ask_not_null_addition', + side_effect=AssertionError("Should not have prompted for not null addition")) + def test_add_date_fields_with_auto_now_not_asking_for_default(self, mocked_ask_method): + # Make state + before = self.make_project_state([self.author_empty]) + after = self.make_project_state([self.author_dates_of_birth_auto_now]) + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes() + # Right number/type of migrations? + self.assertNumberMigrations(changes, 'testapp', 1) + self.assertOperationTypes(changes, 'testapp', 0, ["AddField", "AddField", "AddField"]) + self.assertOperationFieldAttributes(changes, "testapp", 0, 0, auto_now=True) + self.assertOperationFieldAttributes(changes, "testapp", 0, 1, auto_now=True) + self.assertOperationFieldAttributes(changes, "testapp", 0, 2, auto_now=True) + + @mock.patch('django.db.migrations.questioner.MigrationQuestioner.ask_not_null_addition', + side_effect=AssertionError("Should not have prompted for not null addition")) + def test_add_date_fields_with_auto_now_add_not_asking_for_null_addition(self, mocked_ask_method): + # Make state + before = self.make_project_state([self.author_empty]) + after = self.make_project_state([self.author_dates_of_birth_auto_now_add]) + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes() + # Right number/type of migrations? + self.assertNumberMigrations(changes, 'testapp', 1) + self.assertOperationTypes(changes, 'testapp', 0, ["AddField", "AddField", "AddField"]) + self.assertOperationFieldAttributes(changes, "testapp", 0, 0, auto_now_add=True) + self.assertOperationFieldAttributes(changes, "testapp", 0, 1, auto_now_add=True) + self.assertOperationFieldAttributes(changes, "testapp", 0, 2, auto_now_add=True) + + @mock.patch('django.db.migrations.questioner.MigrationQuestioner.ask_auto_now_add_addition') + def test_add_date_fields_with_auto_now_add_asking_for_default(self, mocked_ask_method): + # Make state + before = self.make_project_state([self.author_empty]) + after = self.make_project_state([self.author_dates_of_birth_auto_now_add]) + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes() + # Right number/type of migrations? + self.assertNumberMigrations(changes, 'testapp', 1) + self.assertOperationTypes(changes, 'testapp', 0, ["AddField", "AddField", "AddField"]) + self.assertOperationFieldAttributes(changes, "testapp", 0, 0, auto_now_add=True) + self.assertOperationFieldAttributes(changes, "testapp", 0, 1, auto_now_add=True) + self.assertOperationFieldAttributes(changes, "testapp", 0, 2, auto_now_add=True) + self.assertEqual(mocked_ask_method.call_count, 3) + def test_remove_field(self): """Tests autodetection of removed fields.""" # Make state diff --git a/tests/migrations/test_commands.py b/tests/migrations/test_commands.py index a5f31f08cb..4d1be3f58f 100644 --- a/tests/migrations/test_commands.py +++ b/tests/migrations/test_commands.py @@ -4,6 +4,7 @@ from __future__ import unicode_literals import codecs import importlib import os +import sys from django.apps import apps from django.core.management import CommandError, call_command @@ -1078,6 +1079,30 @@ class MakeMigrationsTests(MigrationTestBase): with self.assertRaisesMessage(InconsistentMigrationHistory, msg): call_command("makemigrations") + @mock.patch('django.db.migrations.questioner.input', return_value='1') + @mock.patch('django.db.migrations.questioner.sys.stdin', mock.MagicMock(encoding=sys.getdefaultencoding())) + def test_makemigrations_auto_now_add_interactive(self, *args): + """ + makemigrations prompts the user when adding auto_now_add to an existing + model. + """ + class Entry(models.Model): + title = models.CharField(max_length=255) + creation_date = models.DateTimeField(auto_now_add=True) + + class Meta: + app_label = 'migrations' + + # Monkeypatch interactive questioner to auto accept + with mock.patch('django.db.migrations.questioner.sys.stdout', new_callable=six.StringIO) as prompt_stdout: + out = six.StringIO() + with self.temporary_migration_module(module='migrations.test_auto_now_add'): + call_command('makemigrations', 'migrations', interactive=True, stdout=out) + output = force_text(out.getvalue()) + prompt_output = force_text(prompt_stdout.getvalue()) + self.assertIn("You can accept the default 'timezone.now' by pressing 'Enter'", prompt_output) + self.assertIn("Add field creation_date to entry", output) + class SquashMigrationsTests(MigrationTestBase): """ diff --git a/tests/schema/tests.py b/tests/schema/tests.py index 5e2bd8ef96..955d63c97d 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -18,8 +18,9 @@ from django.db.models.fields.related import ( ) from django.db.transaction import atomic from django.test import ( - TransactionTestCase, skipIfDBFeature, skipUnlessDBFeature, + TransactionTestCase, mock, skipIfDBFeature, skipUnlessDBFeature, ) +from django.utils.timezone import UTC from .fields import ( CustomManyToManyField, InheritedManyToManyField, MediumBlobField, @@ -124,8 +125,17 @@ class SchemaTests(TransactionTestCase): constraints_for_column.append(name) return sorted(constraints_for_column) - # Tests + def check_added_field_default(self, schema_editor, model, field, field_name, expected_default, + cast_function=None): + with connection.cursor() as cursor: + schema_editor.add_field(model, field) + cursor.execute("SELECT {} FROM {};".format(field_name, model._meta.db_table)) + database_default = cursor.fetchall()[0][0] + if cast_function and not type(database_default) == type(expected_default): + database_default = cast_function(database_default) + self.assertEqual(database_default, expected_default) + # Tests def test_creation_deletion(self): """ Tries creating a model's table, and then deleting it. @@ -1833,3 +1843,63 @@ class SchemaTests(TransactionTestCase): new_field.set_attributes_from_name('id') with connection.schema_editor() as editor: editor.alter_field(Node, old_field, new_field) + + @mock.patch('django.db.backends.base.schema.datetime') + @mock.patch('django.db.backends.base.schema.timezone') + def test_add_datefield_and_datetimefield_use_effective_default(self, mocked_datetime, mocked_tz): + """ + effective_default() should be used for DateField, DateTimeField, and + TimeField if auto_now or auto_add_now is set (#25005). + """ + now = datetime.datetime(month=1, day=1, year=2000, hour=1, minute=1) + now_tz = datetime.datetime(month=1, day=1, year=2000, hour=1, minute=1, tzinfo=UTC()) + mocked_datetime.now = mock.MagicMock(return_value=now) + mocked_tz.now = mock.MagicMock(return_value=now_tz) + # Create the table + with connection.schema_editor() as editor: + editor.create_model(Author) + # Check auto_now/auto_now_add attributes are not defined + columns = self.column_classes(Author) + self.assertNotIn("dob_auto_now", columns) + self.assertNotIn("dob_auto_now_add", columns) + self.assertNotIn("dtob_auto_now", columns) + self.assertNotIn("dtob_auto_now_add", columns) + self.assertNotIn("tob_auto_now", columns) + self.assertNotIn("tob_auto_now_add", columns) + # Create a row + Author.objects.create(name='Anonymous1') + # Ensure fields were added with the correct defaults + dob_auto_now = DateField(auto_now=True) + dob_auto_now.set_attributes_from_name('dob_auto_now') + self.check_added_field_default( + editor, Author, dob_auto_now, 'dob_auto_now', now.date(), + cast_function=lambda x: x.date(), + ) + dob_auto_now_add = DateField(auto_now_add=True) + dob_auto_now_add.set_attributes_from_name('dob_auto_now_add') + self.check_added_field_default( + editor, Author, dob_auto_now_add, 'dob_auto_now_add', now.date(), + cast_function=lambda x: x.date(), + ) + dtob_auto_now = DateTimeField(auto_now=True) + dtob_auto_now.set_attributes_from_name('dtob_auto_now') + self.check_added_field_default( + editor, Author, dtob_auto_now, 'dtob_auto_now', now, + ) + dt_tm_of_birth_auto_now_add = DateTimeField(auto_now_add=True) + dt_tm_of_birth_auto_now_add.set_attributes_from_name('dtob_auto_now_add') + self.check_added_field_default( + editor, Author, dt_tm_of_birth_auto_now_add, 'dtob_auto_now_add', now, + ) + tob_auto_now = TimeField(auto_now=True) + tob_auto_now.set_attributes_from_name('tob_auto_now') + self.check_added_field_default( + editor, Author, tob_auto_now, 'tob_auto_now', now.time(), + cast_function=lambda x: x.time(), + ) + tob_auto_now_add = TimeField(auto_now_add=True) + tob_auto_now_add.set_attributes_from_name('tob_auto_now_add') + self.check_added_field_default( + editor, Author, tob_auto_now_add, 'tob_auto_now_add', now.time(), + cast_function=lambda x: x.time(), + )