From a0c4b8465db3a4b2df247912d79c5c0a752cee76 Mon Sep 17 00:00:00 2001 From: Chris Beaven Date: Fri, 2 May 2014 14:50:56 +1200 Subject: [PATCH] Fix migration autodector to work correctly with custom deconstructed values --- django/db/migrations/autodetector.py | 27 ++++++++++++++++++++++----- tests/migrations/test_autodetector.py | 22 ++++++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index f8b631270f..af1c693f27 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -64,6 +64,23 @@ class MigrationAutodetector(object): if not model._meta.proxy and model._meta.managed and al not in self.to_state.real_apps: new_model_keys.append((al, mn)) + def _deep_deconstruct(obj, field=True): + """ + Recursive deconstruction for a field and its arguments. + """ + if not hasattr(obj, 'deconstruct'): + return obj + deconstructed = obj.deconstruct() + if field: + deconstructed = deconstructed[1:] + name, args, kwargs = deconstructed + return ( + name, + [_deep_deconstruct(value, field=False) for value in args], + dict([(key, _deep_deconstruct(value, field=False)) + for key, value in kwargs.items()]) + ) + def _rel_agnostic_fields_def(fields): """ Return a definition of the fields that ignores field names and @@ -71,7 +88,7 @@ class MigrationAutodetector(object): """ fields_def = [] for name, field in fields: - deconstruction = field.deconstruct()[1:] + deconstruction = _deep_deconstruct(field) if field.rel and field.rel.to: del deconstruction[2]['to'] fields_def.append(deconstruction) @@ -255,11 +272,11 @@ class MigrationAutodetector(object): new_model_state = self.to_state.models[app_label, model_name] field = new_model_state.get_field_by_name(field_name) # Scan to see if this is actually a rename! - field_dec = field.deconstruct()[1:] + field_dec = _deep_deconstruct(field) found_rename = False for rem_app_label, rem_model_name, rem_field_name in (old_fields - new_fields): if rem_app_label == app_label and rem_model_name == model_name: - old_field_dec = old_model_state.get_field_by_name(rem_field_name).deconstruct()[1:] + old_field_dec = _deep_deconstruct(old_model_state.get_field_by_name(rem_field_name)) if field.rel and field.rel.to and 'to' in old_field_dec[2]: old_rel_to = old_field_dec[2]['to'] if old_rel_to in renamed_models_rel: @@ -326,8 +343,8 @@ class MigrationAutodetector(object): old_model_state = self.from_state.models[app_label, old_model_name] new_model_state = self.to_state.models[app_label, model_name] old_field_name = renamed_fields.get((app_label, model_name, field_name), field_name) - old_field_dec = old_model_state.get_field_by_name(old_field_name).deconstruct()[1:] - new_field_dec = new_model_state.get_field_by_name(field_name).deconstruct()[1:] + old_field_dec = _deep_deconstruct(old_model_state.get_field_by_name(old_field_name)) + new_field_dec = _deep_deconstruct(new_model_state.get_field_by_name(field_name)) if old_field_dec != new_field_dec: self.add_to_migration( app_label, diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 2b8e940c4e..c92dda8377 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -7,6 +7,15 @@ from django.db.migrations.graph import MigrationGraph from django.db import models +class DeconstructableObject(object): + """ + A custom deconstructable object. + """ + + def deconstruct(self): + return self.__module__ + '.' + self.__class__.__name__, [], {} + + class AutodetectorTests(TestCase): """ Tests the migration autodetector. @@ -17,6 +26,8 @@ class AutodetectorTests(TestCase): author_name_longer = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=400))]) author_name_renamed = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("names", models.CharField(max_length=200))]) author_name_default = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200, default='Ada Lovelace'))]) + author_name_deconstructable_1 = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200, default=DeconstructableObject()))]) + author_name_deconstructable_2 = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200, default=DeconstructableObject()))]) author_with_book = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)), ("book", models.ForeignKey("otherapp.Book"))]) author_renamed_with_book = ModelState("testapp", "Writer", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)), ("book", models.ForeignKey("otherapp.Book"))]) author_with_publisher_string = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)), ("publisher_name", models.CharField(max_length=200))]) @@ -550,6 +561,17 @@ class AutodetectorTests(TestCase): self.assertEqual(action.__class__.__name__, "AddField") self.assertEqual(action.name, "name") + def test_custom_deconstructable(self): + """ + Two instances which deconstruct to the same value aren't considered a + change. + """ + before = self.make_project_state([self.author_name_deconstructable_1]) + after = self.make_project_state([self.author_name_deconstructable_2]) + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes() + self.assertEqual(changes, {}) + def test_replace_string_with_foreignkey(self): """ Adding an FK in the same "spot" as a deleted CharField should work. (#22300).