From 144cff3f516f7c718cbe4bd3165e18781c09042a Mon Sep 17 00:00:00 2001 From: Markus Holtermann Date: Wed, 20 Aug 2014 16:15:23 +0200 Subject: [PATCH] Fixed #23322 -- Use resolved swappable model for dependency resolution during makemigrations --- django/db/migrations/autodetector.py | 16 +++++- tests/migrations/test_autodetector.py | 82 +++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index e67c8aa902..4a837fbba1 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -237,9 +237,17 @@ class MigrationAutodetector(object): deps_satisfied = True operation_dependencies = set() for dep in operation._auto_deps: + is_swappable_dep = False if dep[0] == "__setting__": - operation_dependencies.add((dep[0], dep[1])) - elif dep[0] != app_label: + # We need to temporarily resolve the swappable dependency to prevent + # circular references. While keeping the dependency checks on the + # resolved model we still add the swappable dependencies. + # See #23322 + resolved_app_label, resolved_object_name = getattr(settings, dep[1]).split('.') + original_dep = dep + dep = (resolved_app_label, resolved_object_name.lower(), dep[2], dep[3]) + is_swappable_dep = True + if dep[0] != app_label and dep[0] != "__setting__": # External app dependency. See if it's not yet # satisfied. for other_operation in self.generated_operations.get(dep[0], []): @@ -249,7 +257,9 @@ class MigrationAutodetector(object): if not deps_satisfied: break else: - if self.migrations.get(dep[0], None): + if is_swappable_dep: + operation_dependencies.add((original_dep[0], original_dep[1])) + elif dep[0] in self.migrations: operation_dependencies.add((dep[0], self.migrations[dep[0]][-1].name)) else: # If we can't find the other app, we add a first/last dependency, diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 1007a9f1c6..1959e129d5 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +from django.conf import settings from django.test import TestCase, override_settings from django.db.migrations.autodetector import MigrationAutodetector from django.db.migrations.questioner import MigrationQuestioner @@ -1103,3 +1104,84 @@ class AutodetectorTests(TestCase): self.assertOperationTypes(changes, 'a', 0, ["CreateModel", "CreateModel"]) self.assertOperationTypes(changes, 'a', 1, ["AddField"]) self.assertOperationTypes(changes, 'b', 0, ["CreateModel", "CreateModel"]) + + @override_settings(AUTH_USER_MODEL="a.Tenant") + def test_circular_dependency_swappable(self): + """ + Tests that the dependency resolver knows to explicitly resolve + swappable models (#23322) + """ + tenant = ModelState("a", "Tenant", [ + ("id", models.AutoField(primary_key=True)), + ("primary_address", models.ForeignKey("b.Address"))], + bases=(AbstractBaseUser, ) + ) + address = ModelState("b", "Address", [ + ("id", models.AutoField(primary_key=True)), + ("tenant", models.ForeignKey(settings.AUTH_USER_MODEL)), + ]) + # Make state + before = self.make_project_state([]) + after = self.make_project_state([address, tenant]) + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes() + # Right number of migrations? + self.assertNumberMigrations(changes, 'a', 2) + self.assertNumberMigrations(changes, 'b', 1) + self.assertOperationTypes(changes, 'a', 0, ["CreateModel"]) + self.assertOperationTypes(changes, 'a', 1, ["AddField"]) + self.assertOperationTypes(changes, 'b', 0, ["CreateModel"]) + self.assertEqual(changes['a'][0].dependencies, []) + self.assertEqual(set(changes['a'][1].dependencies), set([('a', 'auto_1'), ('b', 'auto_1')])) + self.assertEqual(changes['b'][0].dependencies, [('__setting__', 'AUTH_USER_MODEL')]) + + @override_settings(AUTH_USER_MODEL="b.Tenant") + def test_circular_dependency_swappable2(self): + """ + Tests that the dependency resolver knows to explicitly resolve + swappable models but with the swappable not being the first migrated + model (#23322) + """ + address = ModelState("a", "Address", [ + ("id", models.AutoField(primary_key=True)), + ("tenant", models.ForeignKey(settings.AUTH_USER_MODEL)), + ]) + tenant = ModelState("b", "Tenant", [ + ("id", models.AutoField(primary_key=True)), + ("primary_address", models.ForeignKey("a.Address"))], + bases=(AbstractBaseUser, ) + ) + # Make state + before = self.make_project_state([]) + after = self.make_project_state([address, tenant]) + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes() + # Right number of migrations? + self.assertNumberMigrations(changes, 'a', 2) + self.assertNumberMigrations(changes, 'b', 1) + self.assertOperationTypes(changes, 'a', 0, ["CreateModel"]) + self.assertOperationTypes(changes, 'a', 1, ["AddField"]) + self.assertOperationTypes(changes, 'b', 0, ["CreateModel"]) + self.assertEqual(changes['a'][0].dependencies, []) + self.assertEqual(set(changes['a'][1].dependencies), set([('__setting__', 'AUTH_USER_MODEL'), ('a', 'auto_1')])) + self.assertEqual(changes['b'][0].dependencies, [('a', 'auto_1')]) + + @override_settings(AUTH_USER_MODEL="a.Person") + def test_circular_dependency_swappable_self(self): + """ + Tests that the dependency resolver knows to explicitly resolve + swappable models (#23322) + """ + person = ModelState("a", "Person", [ + ("id", models.AutoField(primary_key=True)), + ("parent1", models.ForeignKey(settings.AUTH_USER_MODEL, related_name='children')) + ]) + # Make state + before = self.make_project_state([]) + after = self.make_project_state([person]) + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes() + # Right number of migrations? + self.assertNumberMigrations(changes, 'a', 1) + self.assertOperationTypes(changes, 'a', 0, ["CreateModel"]) + self.assertEqual(changes['a'][0].dependencies, [])