Fixed #24513 -- Made sure a model is only rendered once during reloads

This also prevents state modifications from corrupting previous states.
Previously, when a model defining a relation was unregistered first,
clearing the cache would cause its related models' _meta to be cleared
and would result in the old models losing track of their relations.

django/db/migrations/state.py

@@ -83,6 +83,9 @@ def remove_model(self, app_label, model_name):
         del self.models[app_label, model_name]
         if 'apps' in self.__dict__:  # hasattr would cache the property
             self.apps.unregister_model(app_label, model_name)
+            # Need to do this explicitly since unregister_model() doesn't clear
+            # the cache automatically (#24513)
+            self.apps.clear_cache()
 
     def reload_model(self, app_label, model_name):
         if 'apps' in self.__dict__:  # hasattr would cache the property
@@ -104,29 +107,25 @@ def reload_model(self, app_label, model_name):
                     rel_app_label, rel_model_name = _get_app_label_and_model_name(field.remote_field.model, app_label)
                     related_models.add((rel_app_label, rel_model_name.lower()))
 
+            # Include the model itself
+            related_models.add((app_label, model_name))
+
             # Unregister all related models
             for rel_app_label, rel_model_name in related_models:
                 self.apps.unregister_model(rel_app_label, rel_model_name)
+            # Need to do it once all models are unregistered to avoid corrupting
+            # existing models' _meta
+            self.apps.clear_cache()
 
-            # Unregister the current model
-            self.apps.unregister_model(app_label, model_name)
-
+            states_to_be_rendered = []
             # Gather all models states of those models that will be rerendered.
             # This includes:
-            # 1. The current model
-            try:
-                model_state = self.models[app_label, model_name]
-            except KeyError:
-                states_to_be_rendered = []
-            else:
-                states_to_be_rendered = [model_state]
-
-            # 2. All related models of unmigrated apps
+            # 1. All related models of unmigrated apps
             for model_state in self.apps.real_models:
                 if (model_state.app_label, model_state.name_lower) in related_models:
                     states_to_be_rendered.append(model_state)
 
-            # 3. All related models of migrated apps
+            # 2. All related models of migrated apps
             for rel_app_label, rel_model_name in related_models:
                 try:
                     model_state = self.models[rel_app_label, rel_model_name]
@@ -282,7 +281,6 @@ def unregister_model(self, app_label, model_name):
             del self.app_configs[app_label].models[model_name]
         except KeyError:
             pass
-        self.clear_cache()
 
 
 class ModelState(object):

docs/releases/1.8.1.txt

@@ -25,3 +25,6 @@ Bugfixes
 * Stripped microseconds from ``datetime`` values when using an older version of
   the MySQLdb DB API driver as it does not support fractional seconds
   (:ticket:`24584`).
+
+* Fixed a migration crash when altering
+  :class:`~django.db.models.ManyToManyField`\s (:ticket:`24513`).

tests/migrations/test_state.py

@@ -1,6 +1,8 @@
 from django.apps.registry import Apps
 from django.db import models
-from django.db.migrations.operations import DeleteModel, RemoveField
+from django.db.migrations.operations import (
+    AlterField, DeleteModel, RemoveField,
+)
 from django.db.migrations.state import (
     InvalidBasesError, ModelState, ProjectState, get_related_models_recursive,
 )
@@ -413,6 +415,55 @@ def get_model_a(state):
         self.assertEqual(len(model_a_old._meta.related_objects), 1)
         self.assertEqual(len(model_a_new._meta.related_objects), 0)
 
+    def test_self_relation(self):
+        """
+        #24513 - Modifying an object pointing to itself would cause it to be
+        rendered twice and thus breaking its related M2M through objects.
+        """
+        class A(models.Model):
+            to_a = models.ManyToManyField('something.A', symmetrical=False)
+
+            class Meta:
+                app_label = "something"
+
+        def get_model_a(state):
+            return [mod for mod in state.apps.get_models() if mod._meta.model_name == 'a'][0]
+
+        project_state = ProjectState()
+        project_state.add_model((ModelState.from_model(A)))
+        self.assertEqual(len(get_model_a(project_state)._meta.related_objects), 1)
+        old_state = project_state.clone()
+
+        operation = AlterField(
+            model_name="a",
+            name="to_a",
+            field=models.ManyToManyField("something.A", symmetrical=False, blank=True)
+        )
+        # At this point the model would be rendered twice causing its related
+        # M2M through objects to point to an old copy and thus breaking their
+        # attribute lookup.
+        operation.state_forwards("something", project_state)
+
+        model_a_old = get_model_a(old_state)
+        model_a_new = get_model_a(project_state)
+        self.assertIsNot(model_a_old, model_a_new)
+
+        # Tests that the old model's _meta is still consistent
+        field_to_a_old = model_a_old._meta.get_field("to_a")
+        self.assertEqual(field_to_a_old.m2m_field_name(), "from_a")
+        self.assertEqual(field_to_a_old.m2m_reverse_field_name(), "to_a")
+        self.assertIs(field_to_a_old.related_model, model_a_old)
+        self.assertIs(field_to_a_old.remote_field.through._meta.get_field('to_a').related_model, model_a_old)
+        self.assertIs(field_to_a_old.remote_field.through._meta.get_field('from_a').related_model, model_a_old)
+
+        # Tests that the new model's _meta is still consistent
+        field_to_a_new = model_a_new._meta.get_field("to_a")
+        self.assertEqual(field_to_a_new.m2m_field_name(), "from_a")
+        self.assertEqual(field_to_a_new.m2m_reverse_field_name(), "to_a")
+        self.assertIs(field_to_a_new.related_model, model_a_new)
+        self.assertIs(field_to_a_new.remote_field.through._meta.get_field('to_a').related_model, model_a_new)
+        self.assertIs(field_to_a_new.remote_field.through._meta.get_field('from_a').related_model, model_a_new)
+
     def test_equality(self):
         """
         Tests that == and != are implemented correctly.