Fixed #24340 -- Added nested deconstruction for list, tuple and dict values

Nested deconstruction should recursively deconstruct items within list,
tuple and dict values.

2 files changed, 236 insertions, 15 deletions

diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py
index ad6aeda380..9a4ba42722 100644
--- a/django/db/migrations/autodetector.py
+++ b/django/db/migrations/autodetector.py
@@ -52,21 +52,35 @@ class MigrationAutodetector(object):
         Used for full comparison for rename/alter; sometimes a single-level
         deconstruction will not compare correctly.
         """
-        if not hasattr(obj, 'deconstruct') or isinstance(obj, type):
-            return obj
-        deconstructed = obj.deconstruct()
-        if isinstance(obj, models.Field):
-            # we have a field which also returns a name
-            deconstructed = deconstructed[1:]
-        path, args, kwargs = deconstructed
-        return (
-            path,
-            [self.deep_deconstruct(value) for value in args],
-            {
+        if isinstance(obj, list):
+            return [self.deep_deconstruct(value) for value in obj]
+        elif isinstance(obj, tuple):
+            return tuple(self.deep_deconstruct(value) for value in obj)
+        elif isinstance(obj, dict):
+            return {
                 key: self.deep_deconstruct(value)
-                for key, value in kwargs.items()
-            },
-        )
+                for key, value in obj.items()
+            }
+        elif isinstance(obj, type):
+            # If this is a type that implements 'deconstruct' as an instance method,
+            # avoid treating this as being deconstructible itself - see #22951
+            return obj
+        elif hasattr(obj, 'deconstruct'):
+            deconstructed = obj.deconstruct()
+            if isinstance(obj, models.Field):
+                # we have a field which also returns a name
+                deconstructed = deconstructed[1:]
+            path, args, kwargs = deconstructed
+            return (
+                path,
+                [self.deep_deconstruct(value) for value in args],
+                {
+                    key: self.deep_deconstruct(value)
+                    for key, value in kwargs.items()
+                },
+            )
+        else:
+            return obj
 
     def only_relation_agnostic_fields(self, fields):
         """
diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py
index 269d41fd2c..d5ddd30894 100644
--- a/tests/migrations/test_autodetector.py
+++ b/tests/migrations/test_autodetector.py
@@ -17,8 +17,16 @@ class DeconstructableObject(object):
     A custom deconstructable object.
     """
 
+    def __init__(self, *args, **kwargs):
+        self.args = args
+        self.kwargs = kwargs
+
     def deconstruct(self):
-        return self.__module__ + '.' + self.__class__.__name__, [], {}
+        return (
+            self.__module__ + '.' + self.__class__.__name__,
+            self.args,
+            self.kwargs
+        )
 
 
 class AutodetectorTests(TestCase):
@@ -63,6 +71,104 @@ class AutodetectorTests(TestCase):
         ("id", models.AutoField(primary_key=True)),
         ("name", models.CharField(max_length=200, default=models.IntegerField())),
     ])
+    author_name_deconstructable_list_1 = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default=[DeconstructableObject(), 123])),
+    ])
+    author_name_deconstructable_list_2 = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default=[DeconstructableObject(), 123])),
+    ])
+    author_name_deconstructable_list_3 = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default=[DeconstructableObject(), 999])),
+    ])
+    author_name_deconstructable_tuple_1 = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default=(DeconstructableObject(), 123))),
+    ])
+    author_name_deconstructable_tuple_2 = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default=(DeconstructableObject(), 123))),
+    ])
+    author_name_deconstructable_tuple_3 = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default=(DeconstructableObject(), 999))),
+    ])
+    author_name_deconstructable_dict_1 = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default={
+            'item': DeconstructableObject(), 'otheritem': 123
+        })),
+    ])
+    author_name_deconstructable_dict_2 = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default={
+            'item': DeconstructableObject(), 'otheritem': 123
+        })),
+    ])
+    author_name_deconstructable_dict_3 = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default={
+            'item': DeconstructableObject(), 'otheritem': 999
+        })),
+    ])
+    author_name_nested_deconstructable_1 = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default=DeconstructableObject(
+            DeconstructableObject(1),
+            (DeconstructableObject('t1'), DeconstructableObject('t2'),),
+            a=DeconstructableObject('A'),
+            b=DeconstructableObject(B=DeconstructableObject('c')),
+        ))),
+    ])
+    author_name_nested_deconstructable_2 = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default=DeconstructableObject(
+            DeconstructableObject(1),
+            (DeconstructableObject('t1'), DeconstructableObject('t2'),),
+            a=DeconstructableObject('A'),
+            b=DeconstructableObject(B=DeconstructableObject('c')),
+        ))),
+    ])
+    author_name_nested_deconstructable_changed_arg = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default=DeconstructableObject(
+            DeconstructableObject(1),
+            (DeconstructableObject('t1'), DeconstructableObject('t2-changed'),),
+            a=DeconstructableObject('A'),
+            b=DeconstructableObject(B=DeconstructableObject('c')),
+        ))),
+    ])
+    author_name_nested_deconstructable_extra_arg = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default=DeconstructableObject(
+            DeconstructableObject(1),
+            (DeconstructableObject('t1'), DeconstructableObject('t2'),),
+            None,
+            a=DeconstructableObject('A'),
+            b=DeconstructableObject(B=DeconstructableObject('c')),
+        ))),
+    ])
+    author_name_nested_deconstructable_changed_kwarg = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default=DeconstructableObject(
+            DeconstructableObject(1),
+            (DeconstructableObject('t1'), DeconstructableObject('t2'),),
+            a=DeconstructableObject('A'),
+            b=DeconstructableObject(B=DeconstructableObject('c-changed')),
+        ))),
+    ])
+    author_name_nested_deconstructable_extra_kwarg = ModelState("testapp", "Author", [
+        ("id", models.AutoField(primary_key=True)),
+        ("name", models.CharField(max_length=200, default=DeconstructableObject(
+            DeconstructableObject(1),
+            (DeconstructableObject('t1'), DeconstructableObject('t2'),),
+            a=DeconstructableObject('A'),
+            b=DeconstructableObject(B=DeconstructableObject('c')),
+            c=None,
+        ))),
+    ])
     author_custom_pk = ModelState("testapp", "Author", [("pk_field", models.IntegerField(primary_key=True))])
     author_with_biography_non_blank = ModelState("testapp", "Author", [
         ("id", models.AutoField(primary_key=True)),
@@ -1225,6 +1331,107 @@ class AutodetectorTests(TestCase):
         changes = autodetector._detect_changes()
         self.assertEqual(changes, {})
 
+    def test_deconstructable_list(self):
+        """Nested deconstruction descends into lists."""
+        # When lists contain items that deconstruct to identical values, those lists
+        # should be considered equal for the purpose of detecting state changes
+        # (even if the original items are unequal).
+        before = self.make_project_state([self.author_name_deconstructable_list_1])
+        after = self.make_project_state([self.author_name_deconstructable_list_2])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        self.assertEqual(changes, {})
+
+        # Legitimate differences within the deconstructed lists should be reported
+        # as a change
+        before = self.make_project_state([self.author_name_deconstructable_list_1])
+        after = self.make_project_state([self.author_name_deconstructable_list_3])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        self.assertEqual(len(changes), 1)
+
+    def test_deconstructable_tuple(self):
+        """Nested deconstruction descends into tuples."""
+        # When tuples contain items that deconstruct to identical values, those tuples
+        # should be considered equal for the purpose of detecting state changes
+        # (even if the original items are unequal).
+        before = self.make_project_state([self.author_name_deconstructable_tuple_1])
+        after = self.make_project_state([self.author_name_deconstructable_tuple_2])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        self.assertEqual(changes, {})
+
+        # Legitimate differences within the deconstructed tuples should be reported
+        # as a change
+        before = self.make_project_state([self.author_name_deconstructable_tuple_1])
+        after = self.make_project_state([self.author_name_deconstructable_tuple_3])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        self.assertEqual(len(changes), 1)
+
+    def test_deconstructable_dict(self):
+        """Nested deconstruction descends into dict values."""
+        # When dicts contain items whose values deconstruct to identical values,
+        # those dicts should be considered equal for the purpose of detecting
+        # state changes (even if the original values are unequal).
+        before = self.make_project_state([self.author_name_deconstructable_dict_1])
+        after = self.make_project_state([self.author_name_deconstructable_dict_2])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        self.assertEqual(changes, {})
+
+        # Legitimate differences within the deconstructed dicts should be reported
+        # as a change
+        before = self.make_project_state([self.author_name_deconstructable_dict_1])
+        after = self.make_project_state([self.author_name_deconstructable_dict_3])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        self.assertEqual(len(changes), 1)
+
+    def test_nested_deconstructable_objects(self):
+        """
+        Nested deconstruction is applied recursively to the args/kwargs of
+        deconstructed objects.
+        """
+        # If the items within a deconstructed object's args/kwargs have the same
+        # deconstructed values - whether or not the items themselves are different
+        # instances - then the object as a whole is regarded as unchanged.
+        before = self.make_project_state([self.author_name_nested_deconstructable_1])
+        after = self.make_project_state([self.author_name_nested_deconstructable_2])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        self.assertEqual(changes, {})
+
+        # Differences that exist solely within the args list of a deconstructed object
+        # should be reported as changes
+        before = self.make_project_state([self.author_name_nested_deconstructable_1])
+        after = self.make_project_state([self.author_name_nested_deconstructable_changed_arg])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        self.assertEqual(len(changes), 1)
+
+        # Additional args should also be reported as a change
+        before = self.make_project_state([self.author_name_nested_deconstructable_1])
+        after = self.make_project_state([self.author_name_nested_deconstructable_extra_arg])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        self.assertEqual(len(changes), 1)
+
+        # Differences that exist solely within the kwargs dict of a deconstructed object
+        # should be reported as changes
+        before = self.make_project_state([self.author_name_nested_deconstructable_1])
+        after = self.make_project_state([self.author_name_nested_deconstructable_changed_kwarg])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        self.assertEqual(len(changes), 1)
+
+        # Additional kwargs should also be reported as a change
+        before = self.make_project_state([self.author_name_nested_deconstructable_1])
+        after = self.make_project_state([self.author_name_nested_deconstructable_extra_kwarg])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        self.assertEqual(len(changes), 1)
+
     def test_deconstruct_type(self):
         """
         #22951 -- Uninstanted classes with deconstruct are correctly returned