Commit 0385dad0 authored by Patryk Zawadzki's avatar Patryk Zawadzki Committed by Markus Holtermann
Browse files

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.
parent 7a7c7972

django/db/migrations/state.py

+12 −14
Original line number Diff line number Diff line
@@ -83,6 +83,9 @@ class ProjectState(object): 
        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 @@ class ProjectState(object): 
                    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 @@ class StateApps(Apps): 
            del self.app_configs[app_label].models[model_name]

        except KeyError:

            pass

        self.clear_cache()





class ModelState(object):

docs/releases/1.8.1.txt

+3 −0
Original line number Diff line number Diff line
@@ -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

+52 −1
Original line number Diff line number Diff line 
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 @@ class StateTests(TestCase): 
        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.