Browse Source

Fixed #27666 -- Delayed rendering of recursivly related models in migration operations.

Markus Holtermann 8 years ago
parent
commit
45ded053b1

+ 3 - 0
django/core/management/commands/migrate.py

@@ -203,6 +203,9 @@ class Command(BaseCommand):
             targets, plan=plan, state=pre_migrate_state.clone(), fake=fake,
             fake_initial=fake_initial,
         )
+        # post_migrate signals have access to all models. Ensure that all models
+        # are reloaded in case any are delayed.
+        post_migrate_state.clear_delayed_apps_cache()
         post_migrate_apps = post_migrate_state.apps
 
         # Re-render models of real apps to include relationships now that

+ 21 - 4
django/db/migrations/operations/fields.py

@@ -70,7 +70,9 @@ class AddField(FieldOperation):
         else:
             field = self.field
         state.models[app_label, self.model_name_lower].fields.append((self.name, field))
-        state.reload_model(app_label, self.model_name_lower)
+        # Delay rendering of relationships if it's not a relational field
+        delay = not field.is_relation
+        state.reload_model(app_label, self.model_name_lower, delay=delay)
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         to_model = to_state.apps.get_model(app_label, self.model_name)
@@ -135,11 +137,16 @@ class RemoveField(FieldOperation):
 
     def state_forwards(self, app_label, state):
         new_fields = []
+        old_field = None
         for name, instance in state.models[app_label, self.model_name_lower].fields:
             if name != self.name:
                 new_fields.append((name, instance))
+            else:
+                old_field = instance
         state.models[app_label, self.model_name_lower].fields = new_fields
-        state.reload_model(app_label, self.model_name_lower)
+        # Delay rendering of relationships if it's not a relational field
+        delay = not old_field.is_relation
+        state.reload_model(app_label, self.model_name_lower, delay=delay)
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         from_model = from_state.apps.get_model(app_label, self.model_name)
@@ -191,7 +198,11 @@ class AlterField(FieldOperation):
             for n, f in
             state.models[app_label, self.model_name_lower].fields
         ]
-        state.reload_model(app_label, self.model_name_lower)
+        # TODO: investigate if old relational fields must be reloaded or if it's
+        # sufficient if the new field is (#27737).
+        # Delay rendering of relationships if it's not a relational field
+        delay = not field.is_relation
+        state.reload_model(app_label, self.model_name_lower, delay=delay)
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         to_model = to_state.apps.get_model(app_label, self.model_name)
@@ -270,7 +281,13 @@ class RenameField(FieldOperation):
                     [self.new_name if n == self.old_name else n for n in together]
                     for together in options[option]
                 ]
-        state.reload_model(app_label, self.model_name_lower)
+        for n, f in state.models[app_label, self.model_name_lower].fields:
+            if n == self.new_name:
+                field = f
+                break
+        # Delay rendering of relationships if it's not a relational field
+        delay = not field.is_relation
+        state.reload_model(app_label, self.model_name_lower, delay=delay)
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         to_model = to_state.apps.get_model(app_label, self.model_name)

+ 8 - 8
django/db/migrations/operations/models.py

@@ -331,10 +331,10 @@ class RenameModel(ModelOperation):
                     model_state.fields[index] = name, changed_field
                     model_changed = True
             if model_changed:
-                state.reload_model(model_app_label, model_name)
+                state.reload_model(model_app_label, model_name, delay=True)
         # Remove the old model.
         state.remove_model(app_label, self.old_name_lower)
-        state.reload_model(app_label, self.new_name_lower)
+        state.reload_model(app_label, self.new_name_lower, delay=True)
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         new_model = to_state.apps.get_model(app_label, self.new_name)
@@ -444,7 +444,7 @@ class AlterModelTable(ModelOperation):
 
     def state_forwards(self, app_label, state):
         state.models[app_label, self.name_lower].options["db_table"] = self.table
-        state.reload_model(app_label, self.name_lower)
+        state.reload_model(app_label, self.name_lower, delay=True)
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         new_model = to_state.apps.get_model(app_label, self.name)
@@ -521,7 +521,7 @@ class AlterUniqueTogether(FieldRelatedOptionOperation):
     def state_forwards(self, app_label, state):
         model_state = state.models[app_label, self.name_lower]
         model_state.options[self.option_name] = self.unique_together
-        state.reload_model(app_label, self.name_lower)
+        state.reload_model(app_label, self.name_lower, delay=True)
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         new_model = to_state.apps.get_model(app_label, self.name)
@@ -575,7 +575,7 @@ class AlterIndexTogether(FieldRelatedOptionOperation):
     def state_forwards(self, app_label, state):
         model_state = state.models[app_label, self.name_lower]
         model_state.options[self.option_name] = self.index_together
-        state.reload_model(app_label, self.name_lower)
+        state.reload_model(app_label, self.name_lower, delay=True)
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         new_model = to_state.apps.get_model(app_label, self.name)
@@ -626,7 +626,7 @@ class AlterOrderWithRespectTo(FieldRelatedOptionOperation):
     def state_forwards(self, app_label, state):
         model_state = state.models[app_label, self.name_lower]
         model_state.options['order_with_respect_to'] = self.order_with_respect_to
-        state.reload_model(app_label, self.name_lower)
+        state.reload_model(app_label, self.name_lower, delay=True)
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         to_model = to_state.apps.get_model(app_label, self.name)
@@ -705,7 +705,7 @@ class AlterModelOptions(ModelOptionOperation):
         for key in self.ALTER_OPTION_KEYS:
             if key not in self.options and key in model_state.options:
                 del model_state.options[key]
-        state.reload_model(app_label, self.name_lower)
+        state.reload_model(app_label, self.name_lower, delay=True)
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         pass
@@ -738,7 +738,7 @@ class AlterModelManagers(ModelOptionOperation):
     def state_forwards(self, app_label, state):
         model_state = state.models[app_label, self.name_lower]
         model_state.managers = list(self.managers)
-        state.reload_model(app_label, self.name_lower)
+        state.reload_model(app_label, self.name_lower, delay=True)
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
         pass

+ 3 - 0
django/db/migrations/operations/special.py

@@ -181,6 +181,9 @@ class RunPython(Operation):
         pass
 
     def database_forwards(self, app_label, schema_editor, from_state, to_state):
+        # RunPython has access to all models. Ensure that all models are
+        # reloaded in case any are delayed.
+        from_state.clear_delayed_apps_cache()
         if router.allow_migrate(schema_editor.connection.alias, app_label, **self.hints):
             # We now execute the Python code in a context that contains a 'models'
             # object, representing the versioned models as an app registry.

+ 29 - 3
django/db/migrations/state.py

@@ -52,6 +52,17 @@ def _get_related_models(m):
     return related_models
 
 
+def get_related_models_tuples(model):
+    """
+    Return a list of typical (app_label, model_name) tuples for all related
+    models for the given model.
+    """
+    return {
+        (rel_mod._meta.app_label, rel_mod._meta.model_name)
+        for rel_mod in _get_related_models(model)
+    }
+
+
 def get_related_models_recursive(model):
     """
     Return all models that have a direct or indirect relationship
@@ -85,6 +96,7 @@ class ProjectState(object):
         self.models = models or {}
         # Apps to include from main registry, usually unmigrated ones
         self.real_apps = real_apps or []
+        self.is_delayed = False
 
     def add_model(self, model_state):
         app_label, model_name = model_state.app_label, model_state.name_lower
@@ -100,7 +112,10 @@ class ProjectState(object):
             # the cache automatically (#24513)
             self.apps.clear_cache()
 
-    def reload_model(self, app_label, model_name):
+    def reload_model(self, app_label, model_name, delay=False):
+        if delay:
+            self.is_delayed = True
+
         if 'apps' in self.__dict__:  # hasattr would cache the property
             try:
                 old_model = self.apps.get_model(app_label, model_name)
@@ -109,7 +124,10 @@ class ProjectState(object):
             else:
                 # Get all relations to and from the old model before reloading,
                 # as _meta.apps may change
-                related_models = get_related_models_recursive(old_model)
+                if delay:
+                    related_models = get_related_models_tuples(old_model)
+                else:
+                    related_models = get_related_models_recursive(old_model)
 
             # Get all outgoing references from the model to be rendered
             model_state = self.models[(app_label, model_name)]
@@ -131,7 +149,10 @@ class ProjectState(object):
                 except LookupError:
                     pass
                 else:
-                    related_models.update(get_related_models_recursive(rel_model))
+                    if delay:
+                        related_models.update(get_related_models_tuples(rel_model))
+                    else:
+                        related_models.update(get_related_models_recursive(rel_model))
 
             # Include the model itself
             related_models.add((app_label, model_name))
@@ -169,8 +190,13 @@ class ProjectState(object):
         )
         if 'apps' in self.__dict__:
             new_state.apps = self.apps.clone()
+        new_state.is_delayed = self.is_delayed
         return new_state
 
+    def clear_delayed_apps_cache(self):
+        if self.is_delayed and 'apps' in self.__dict__:
+            del self.__dict__['apps']
+
     @cached_property
     def apps(self):
         return StateApps(self.real_apps, self.models)

+ 17 - 0
docs/ref/migration-operations.txt

@@ -421,6 +421,8 @@ It accepts two list of operations, and when asked to apply state will use the
 state list, and when asked to apply changes to the database will use the database
 list. Do not use this operation unless you're very sure you know what you're doing.
 
+.. _writing-your-own-migration-operation:
+
 Writing your own
 ================
 
@@ -480,6 +482,21 @@ Some things to note:
   to them; these just represent the difference the ``state_forwards`` method
   would have applied, but are given to you for convenience and speed reasons.
 
+* If you want to work with model classes or model instances from the
+  ``from_state`` argument in ``database_forwards()`` or
+  ``database_backwards()``, you must render model states using the
+  ``clear_delayed_apps_cache()`` method to make related models available::
+
+    def database_forwards(self, app_label, schema_editor, from_state, to_state):
+        # This operation should have access to all models. Ensure that all models are
+        # reloaded in case any are delayed.
+        from_state.clear_delayed_apps_cache()
+        ...
+
+  .. versionadded:: 1.11
+
+    This requirement and the ``clear_delayed_apps_cache()`` method is new.
+
 * ``to_state`` in the database_backwards method is the *older* state; that is,
   the one that will be the current state once the migration has finished reversing.
 

+ 11 - 0
docs/releases/1.11.txt

@@ -593,6 +593,17 @@ must receive a dictionary of context rather than ``Context`` or
 dictionary instead -- doing so is backwards-compatible with older versions of
 Django.
 
+Model state changes in migration operations
+-------------------------------------------
+
+To improve the speed of applying migrations, rendering of related models is
+delayed until an operation that needs them (e.g. ``RunPython``). If you have a
+custom operation that works with model classes or model instances from the
+``from_state`` argument in ``database_forwards()`` or ``database_backwards()``,
+you must render model states using the ``clear_delayed_apps_cache()`` method as
+described in :ref:`writing your own migration operation
+<writing-your-own-migration-operation>`.
+
 Miscellaneous
 -------------