Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

[1.7.x] Fixed #22970: Incorrect dependencies for existing migrated apps

  • Loading branch information...
commit aba75e73db6a0baca1b721698ca24f026bb4a745 1 parent a3d710a
@andrewgodwin andrewgodwin authored
View
20 django/db/migrations/autodetector.py
@@ -47,7 +47,7 @@ def changes(self, graph, trim_to_apps=None, convert_apps=None):
Takes a graph to base names on and an optional set of apps
to try and restrict to (restriction is not guaranteed)
"""
- changes = self._detect_changes(convert_apps)
+ changes = self._detect_changes(convert_apps, graph)
changes = self.arrange_for_graph(changes, graph)
if trim_to_apps:
changes = self._trim_to_apps(changes, trim_to_apps)
@@ -90,7 +90,7 @@ def only_relation_agnostic_fields(self, fields):
fields_def.append(deconstruction)
return fields_def
- def _detect_changes(self, convert_apps=None):
+ def _detect_changes(self, convert_apps=None, graph=None):
"""
Returns a dict of migration plans which will achieve the
change from from_state to to_state. The dict has app labels
@@ -98,6 +98,12 @@ def _detect_changes(self, convert_apps=None):
The resulting migrations aren't specially named, but the names
do matter for dependencies inside the set.
+
+ convert_apps is the list of apps to convert to use migrations
+ (i.e. to make initial migrations for, in the usual case)
+
+ graph is an optional argument that, if provided, can help improve
+ dependency generation and avoid potential circular dependencies.
"""
# The first phase is generating all the operations for each app
@@ -245,10 +251,16 @@ def _detect_changes(self, convert_apps=None):
if self.migrations.get(dep[0], None):
operation_dependencies.add((dep[0], self.migrations[dep[0]][-1].name))
else:
- # If we can't find the other app, we add a __first__ dependency,
+ # If we can't find the other app, we add a first/last dependency,
# but only if we've already been through once and checked everything
if chop_mode:
- operation_dependencies.add((dep[0], "__first__"))
+ # If the app already exists, we add __last__, as we don't know which
+ # migration contains the target field.
+ # If it's not yet migrated or has no migrations, we use __first__
+ if graph and not graph.root_nodes(dep[0]):
+ operation_dependencies.add((dep[0], "__first__"))
+ else:
+ operation_dependencies.add((dep[0], "__last__"))
else:
deps_satisfied = False
if deps_satisfied:
View
8 tests/migrations/models.py
@@ -42,3 +42,11 @@ class UnserializableModel(models.Model):
class Meta:
# Disable auto loading of this model as we load it on our own
apps = Apps()
+
+
+class UnmigratedModel(models.Model):
+ """
+ A model that is in a migration-less app (which this app is
+ if its migrations directory has not been repointed)
+ """
+ pass
View
43 tests/migrations/test_autodetector.py
@@ -4,7 +4,8 @@
from django.db.migrations.questioner import MigrationQuestioner
from django.db.migrations.state import ProjectState, ModelState
from django.db.migrations.graph import MigrationGraph
-from django.db import models
+from django.db.migrations.loader import MigrationLoader
+from django.db import models, connection
from django.contrib.auth.models import AbstractBaseUser
@@ -56,6 +57,7 @@ class AutodetectorTests(TestCase):
third_thing = ModelState("thirdapp", "Thing", [("id", models.AutoField(primary_key=True))])
book = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))])
book_proxy_fk = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("thirdapp.AuthorProxy")), ("title", models.CharField(max_length=200))])
+ book_migrations_fk = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("migrations.UnmigratedModel")), ("title", models.CharField(max_length=200))])
book_with_no_author = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("title", models.CharField(max_length=200))])
book_with_author_renamed = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Writer")), ("title", models.CharField(max_length=200))])
book_with_field_and_author_renamed = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("writer", models.ForeignKey("testapp.Writer")), ("title", models.CharField(max_length=200))])
@@ -1005,3 +1007,42 @@ def test_pk_fk_included(self):
self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel", "CreateModel"])
self.assertOperationAttributes(changes, 'testapp', 0, 0, name="Author")
self.assertOperationAttributes(changes, 'testapp', 0, 1, name="Aardvark")
+
+ def test_first_dependency(self):
+ """
+ Tests that a dependency to an app with no migrations uses __first__.
+ """
+ # Load graph
+ loader = MigrationLoader(connection)
+ # Make state
+ before = self.make_project_state([])
+ after = self.make_project_state([self.book_migrations_fk])
+ after.real_apps = ["migrations"]
+ autodetector = MigrationAutodetector(before, after)
+ changes = autodetector._detect_changes(graph=loader.graph)
+ # Right number of migrations?
+ self.assertNumberMigrations(changes, 'otherapp', 1)
+ self.assertOperationTypes(changes, 'otherapp', 0, ["CreateModel"])
+ self.assertOperationAttributes(changes, 'otherapp', 0, 0, name="Book")
+ # Right dependencies?
+ self.assertEqual(changes['otherapp'][0].dependencies, [("migrations", "__first__")])
+
+ @override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations"})
+ def test_last_dependency(self):
+ """
+ Tests that a dependency to an app with existing migrations uses __first__.
+ """
+ # Load graph
+ loader = MigrationLoader(connection)
+ # Make state
+ before = self.make_project_state([])
+ after = self.make_project_state([self.book_migrations_fk])
+ after.real_apps = ["migrations"]
+ autodetector = MigrationAutodetector(before, after)
+ changes = autodetector._detect_changes(graph=loader.graph)
+ # Right number of migrations?
+ self.assertNumberMigrations(changes, 'otherapp', 1)
+ self.assertOperationTypes(changes, 'otherapp', 0, ["CreateModel"])
+ self.assertOperationAttributes(changes, 'otherapp', 0, 0, name="Book")
+ # Right dependencies?
+ self.assertEqual(changes['otherapp'][0].dependencies, [("migrations", "__last__")])
View
11 tests/migrations/test_migrations_no_changes/0003_third.py
@@ -16,8 +16,15 @@ class Migration(migrations.Migration):
fields=[
('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
],
- options={
- },
+ options={},
+ bases=(models.Model,),
+ ),
+ migrations.CreateModel(
+ name='UnmigratedModel',
+ fields=[
+ ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+ ],
+ options={},
bases=(models.Model,),
),
migrations.DeleteModel(

0 comments on commit aba75e7

Please sign in to comment.
Something went wrong with that request. Please try again.