Fixed #22485: Include all unmigrated apps in project state by default.

This commit is contained in:
Andrew Godwin 2014-04-30 12:25:12 -07:00
parent 24ec9538b7
commit 8f6dff372b
9 changed files with 93 additions and 55 deletions

View File

@ -54,7 +54,7 @@ class Command(BaseCommand):
# (makemigrations doesn't look at the database state). # (makemigrations doesn't look at the database state).
# Also make sure the graph is built without unmigrated apps shoehorned in. # Also make sure the graph is built without unmigrated apps shoehorned in.
loader = MigrationLoader(connections[DEFAULT_DB_ALIAS], load=False) loader = MigrationLoader(connections[DEFAULT_DB_ALIAS], load=False)
loader.build_graph(ignore_unmigrated=True) loader.build_graph()
# Before anything else, see if there's conflicting apps and drop out # Before anything else, see if there's conflicting apps and drop out
# hard if there are any and they don't want to merge # hard if there are any and they don't want to merge
@ -78,7 +78,7 @@ class Command(BaseCommand):
# Set up autodetector # Set up autodetector
autodetector = MigrationAutodetector( autodetector = MigrationAutodetector(
loader.graph.project_state(), loader.project_state(),
ProjectState.from_apps(apps), ProjectState.from_apps(apps),
InteractiveMigrationQuestioner(specified_apps=app_labels), InteractiveMigrationQuestioner(specified_apps=app_labels),
) )

View File

@ -135,7 +135,7 @@ class Command(BaseCommand):
self.stdout.write(" No migrations needed.") self.stdout.write(" No migrations needed.")
# If there's changes that aren't in migrations yet, tell them how to fix it. # If there's changes that aren't in migrations yet, tell them how to fix it.
autodetector = MigrationAutodetector( autodetector = MigrationAutodetector(
executor.loader.graph.project_state(), executor.loader.project_state(),
ProjectState.from_apps(apps), ProjectState.from_apps(apps),
) )
changes = autodetector.changes(graph=executor.loader.graph) changes = autodetector.changes(graph=executor.loader.graph)

View File

@ -50,17 +50,18 @@ class MigrationAutodetector(object):
old_apps = self.from_state.render() old_apps = self.from_state.render()
new_apps = self.to_state.render() new_apps = self.to_state.render()
# Prepare lists of old/new model keys that we care about # Prepare lists of old/new model keys that we care about
# (i.e. ignoring proxy ones) # (i.e. ignoring proxy ones and unmigrated ones)
old_model_keys = [] old_model_keys = []
for al, mn in self.from_state.models.keys(): for al, mn in self.from_state.models.keys():
model = old_apps.get_model(al, mn) model = old_apps.get_model(al, mn)
if not model._meta.proxy and model._meta.managed: if not model._meta.proxy and model._meta.managed and al not in self.from_state.real_apps:
old_model_keys.append((al, mn)) old_model_keys.append((al, mn))
new_model_keys = [] new_model_keys = []
for al, mn in self.to_state.models.keys(): for al, mn in self.to_state.models.keys():
model = new_apps.get_model(al, mn) model = new_apps.get_model(al, mn)
if not model._meta.proxy and model._meta.managed: if not model._meta.proxy and model._meta.managed and al not in self.to_state.real_apps:
new_model_keys.append((al, mn)) new_model_keys.append((al, mn))
def _rel_agnostic_fields_def(fields): def _rel_agnostic_fields_def(fields):

View File

@ -69,7 +69,7 @@ class MigrationExecutor(object):
statements = [] statements = []
for migration, backwards in plan: for migration, backwards in plan:
with self.connection.schema_editor(collect_sql=True) as schema_editor: with self.connection.schema_editor(collect_sql=True) as schema_editor:
project_state = self.loader.graph.project_state((migration.app_label, migration.name), at_end=False) project_state = self.loader.project_state((migration.app_label, migration.name), at_end=False)
if not backwards: if not backwards:
migration.apply(project_state, schema_editor, collect_sql=True) migration.apply(project_state, schema_editor, collect_sql=True)
else: else:
@ -90,7 +90,7 @@ class MigrationExecutor(object):
else: else:
# Alright, do it normally # Alright, do it normally
with self.connection.schema_editor() as schema_editor: with self.connection.schema_editor() as schema_editor:
project_state = self.loader.graph.project_state((migration.app_label, migration.name), at_end=False) project_state = self.loader.project_state((migration.app_label, migration.name), at_end=False)
migration.apply(project_state, schema_editor) migration.apply(project_state, schema_editor)
# For replacement migrations, record individual statuses # For replacement migrations, record individual statuses
if migration.replaces: if migration.replaces:
@ -110,7 +110,7 @@ class MigrationExecutor(object):
self.progress_callback("unapply_start", migration, fake) self.progress_callback("unapply_start", migration, fake)
if not fake: if not fake:
with self.connection.schema_editor() as schema_editor: with self.connection.schema_editor() as schema_editor:
project_state = self.loader.graph.project_state((migration.app_label, migration.name), at_end=False) project_state = self.loader.project_state((migration.app_label, migration.name), at_end=False)
migration.unapply(project_state, schema_editor) migration.unapply(project_state, schema_editor)
# For replacement migrations, record individual statuses # For replacement migrations, record individual statuses
if migration.replaces: if migration.replaces:
@ -128,7 +128,7 @@ class MigrationExecutor(object):
tables it would create exist. This is intended only for use tables it would create exist. This is intended only for use
on initial migrations (as it only looks for CreateModel). on initial migrations (as it only looks for CreateModel).
""" """
project_state = self.loader.graph.project_state((migration.app_label, migration.name), at_end=True) project_state = self.loader.project_state((migration.app_label, migration.name), at_end=True)
apps = project_state.render() apps = project_state.render()
for operation in migration.operations: for operation in migration.operations:
if isinstance(operation, migrations.CreateModel): if isinstance(operation, migrations.CreateModel):

View File

@ -121,7 +121,7 @@ class MigrationGraph(object):
def __str__(self): def __str__(self):
return "Graph: %s nodes, %s edges" % (len(self.nodes), sum(len(x) for x in self.dependencies.values())) return "Graph: %s nodes, %s edges" % (len(self.nodes), sum(len(x) for x in self.dependencies.values()))
def project_state(self, nodes=None, at_end=True): def make_state(self, nodes=None, at_end=True, real_apps=None):
""" """
Given a migration node or nodes, returns a complete ProjectState for it. Given a migration node or nodes, returns a complete ProjectState for it.
If at_end is False, returns the state before the migration has run. If at_end is False, returns the state before the migration has run.
@ -140,7 +140,7 @@ class MigrationGraph(object):
if not at_end and migration in nodes: if not at_end and migration in nodes:
continue continue
plan.append(migration) plan.append(migration)
project_state = ProjectState() project_state = ProjectState(real_apps=real_apps)
for node in plan: for node in plan:
project_state = self.nodes[node].mutate_state(project_state) project_state = self.nodes[node].mutate_state(project_state)
return project_state return project_state

View File

@ -135,7 +135,7 @@ class MigrationLoader(object):
else: else:
return self.disk_migrations[results[0]] return self.disk_migrations[results[0]]
def build_graph(self, ignore_unmigrated=False): def build_graph(self):
""" """
Builds a migration dependency graph using both the disk and database. Builds a migration dependency graph using both the disk and database.
You'll need to rebuild the graph if you apply migrations. This isn't You'll need to rebuild the graph if you apply migrations. This isn't
@ -200,31 +200,10 @@ class MigrationLoader(object):
# even have migrations. # even have migrations.
if parent[1] == "__first__" and parent not in self.graph: if parent[1] == "__first__" and parent not in self.graph:
if parent[0] in self.unmigrated_apps: if parent[0] in self.unmigrated_apps:
if ignore_unmigrated: # This app isn't migrated, but something depends on it.
parent = None # The models will get auto-added into the state, though
else: # so we're fine.
# This app isn't migrated, but something depends on it. continue
# We'll add a fake initial migration for it into the
# graph.
app_config = apps.get_app_config(parent[0])
ops = []
for model in app_config.get_models():
model_state = ModelState.from_model(model)
ops.append(
operations.CreateModel(
name=model_state.name,
fields=model_state.fields,
options=model_state.options,
bases=model_state.bases,
)
)
new_migration = type(
"FakeInitialMigration",
(Migration, ),
{"operations": ops},
)(parent[1], parent[0])
self.graph.add_node(parent, new_migration)
self.applied_migrations.add(parent)
elif parent[0] in self.migrated_apps: elif parent[0] in self.migrated_apps:
parent = list(self.graph.root_nodes(parent[0]))[0] parent = list(self.graph.root_nodes(parent[0]))[0]
else: else:
@ -246,6 +225,15 @@ class MigrationLoader(object):
seen_apps.setdefault(app_label, set()).add(migration_name) seen_apps.setdefault(app_label, set()).add(migration_name)
return dict((app_label, seen_apps[app_label]) for app_label in conflicting_apps) return dict((app_label, seen_apps[app_label]) for app_label in conflicting_apps)
def project_state(self, nodes=None, at_end=True):
"""
Returns a ProjectState object representing the most recent state
that the migrations we loaded represent.
See graph.make_state for the meaning of "nodes" and "at_end"
"""
return self.graph.make_state(nodes=nodes, at_end=at_end, real_apps=list(self.unmigrated_apps))
class BadMigrationError(Exception): class BadMigrationError(Exception):
""" """

View File

@ -1,7 +1,8 @@
from django.apps import AppConfig from django.apps import AppConfig
from django.apps.registry import Apps from django.apps.registry import Apps, apps as global_apps
from django.db import models from django.db import models
from django.db.models.options import DEFAULT_NAMES, normalize_together from django.db.models.options import DEFAULT_NAMES, normalize_together
from django.db.models.fields.related import do_pending_lookups
from django.utils import six from django.utils import six
from django.utils.module_loading import import_string from django.utils.module_loading import import_string
@ -17,9 +18,11 @@ class ProjectState(object):
app level so that cross-app FKs/etc. resolve properly. app level so that cross-app FKs/etc. resolve properly.
""" """
def __init__(self, models=None): def __init__(self, models=None, real_apps=None):
self.models = models or {} self.models = models or {}
self.apps = None self.apps = None
# Apps to include from main registry, usually unmigrated ones
self.real_apps = real_apps or []
def add_model_state(self, model_state): def add_model_state(self, model_state):
self.models[(model_state.app_label, model_state.name.lower())] = model_state self.models[(model_state.app_label, model_state.name.lower())] = model_state
@ -27,20 +30,29 @@ class ProjectState(object):
def clone(self): def clone(self):
"Returns an exact copy of this ProjectState" "Returns an exact copy of this ProjectState"
return ProjectState( return ProjectState(
models=dict((k, v.clone()) for k, v in self.models.items()) models=dict((k, v.clone()) for k, v in self.models.items()),
real_apps=self.real_apps,
) )
def render(self): def render(self, include_real=None):
"Turns the project state into actual models in a new Apps" "Turns the project state into actual models in a new Apps"
if self.apps is None: if self.apps is None:
# Any apps in self.real_apps should have all their models included
# in the render. We don't use the original model instances as there
# are some variables that refer to the Apps object.
real_models = []
for app_label in self.real_apps:
app = global_apps.get_app_config(app_label)
for model in app.get_models():
real_models.append(ModelState.from_model(model))
# Populate the app registry with a stub for each application. # Populate the app registry with a stub for each application.
app_labels = set(model_state.app_label for model_state in self.models.values()) app_labels = set(model_state.app_label for model_state in self.models.values())
self.apps = Apps([AppConfigStub(label) for label in sorted(app_labels)]) self.apps = Apps([AppConfigStub(label) for label in sorted(self.real_apps + list(app_labels))])
# We keep trying to render the models in a loop, ignoring invalid # We keep trying to render the models in a loop, ignoring invalid
# base errors, until the size of the unrendered models doesn't # base errors, until the size of the unrendered models doesn't
# decrease by at least one, meaning there's a base dependency loop/ # decrease by at least one, meaning there's a base dependency loop/
# missing base. # missing base.
unrendered_models = list(self.models.values()) unrendered_models = list(self.models.values()) + real_models
while unrendered_models: while unrendered_models:
new_unrendered_models = [] new_unrendered_models = []
for model in unrendered_models: for model in unrendered_models:
@ -53,14 +65,22 @@ class ProjectState(object):
unrendered_models = new_unrendered_models unrendered_models = new_unrendered_models
# make sure apps has no dangling references # make sure apps has no dangling references
if self.apps._pending_lookups: if self.apps._pending_lookups:
# Raise an error with a best-effort helpful message # There's some lookups left. See if we can first resolve them
# (only for the first issue). Error message should look like: # ourselves - sometimes fields are added after class_prepared is sent
# "ValueError: Lookup failed for model referenced by for lookup_model, operations in self.apps._pending_lookups.items():
# field migrations.Book.author: migrations.Author" try:
dangling_lookup = list(self.apps._pending_lookups.items())[0] model = self.apps.get_model(lookup_model[0], lookup_model[1])
raise ValueError("Lookup failed for model referenced by field {field}: {model[0]}.{model[1]}".format( except LookupError:
field=dangling_lookup[1][0][1], # Raise an error with a best-effort helpful message
model=dangling_lookup[0])) # (only for the first issue). Error message should look like:
# "ValueError: Lookup failed for model referenced by
# field migrations.Book.author: migrations.Author"
raise ValueError("Lookup failed for model referenced by field {field}: {model[0]}.{model[1]}".format(
field=operations[0][1],
model=lookup_model
))
else:
do_pending_lookups(model)
return self.apps return self.apps
@classmethod @classmethod
@ -75,6 +95,8 @@ class ProjectState(object):
def __eq__(self, other): def __eq__(self, other):
if set(self.models.keys()) != set(other.models.keys()): if set(self.models.keys()) != set(other.models.keys()):
return False return False
if set(self.real_apps) != set(other.real_apps):
return False
return all(model == other.models[key] for key, model in self.models.items()) return all(model == other.models[key] for key, model in self.models.items())
def __ne__(self, other): def __ne__(self, other):

View File

@ -61,7 +61,7 @@ class LoaderTests(TestCase):
], ],
) )
# Now render it out! # Now render it out!
project_state = migration_loader.graph.project_state(("migrations", "0002_second")) project_state = migration_loader.project_state(("migrations", "0002_second"))
self.assertEqual(len(project_state.models), 2) self.assertEqual(len(project_state.models), 2)
author_state = project_state.models["migrations", "author"] author_state = project_state.models["migrations", "author"]
@ -76,6 +76,9 @@ class LoaderTests(TestCase):
["id", "author"] ["id", "author"]
) )
# Ensure we've included unmigrated apps in there too
self.assertIn("contenttypes", project_state.real_apps)
@override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations_unmigdep"}) @override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations_unmigdep"})
def test_load_unmigrated_dependency(self): def test_load_unmigrated_dependency(self):
""" """
@ -86,12 +89,11 @@ class LoaderTests(TestCase):
self.assertEqual( self.assertEqual(
migration_loader.graph.forwards_plan(("migrations", "0001_initial")), migration_loader.graph.forwards_plan(("migrations", "0001_initial")),
[ [
("auth", "__first__"),
("migrations", "0001_initial"), ("migrations", "0001_initial"),
], ],
) )
# Now render it out! # Now render it out!
project_state = migration_loader.graph.project_state(("migrations", "0001_initial")) project_state = migration_loader.project_state(("migrations", "0001_initial"))
self.assertEqual(len([m for a, m in project_state.models if a == "migrations"]), 1) self.assertEqual(len([m for a, m in project_state.models if a == "migrations"]), 1)
book_state = project_state.models["migrations", "book"] book_state = project_state.models["migrations", "book"]

View File

@ -338,6 +338,31 @@ class StateTests(TestCase):
with self.assertRaises(ValueError): with self.assertRaises(ValueError):
rendered_state = project_state.render() rendered_state = project_state.render()
def test_real_apps(self):
"""
Tests that including real apps can resolve dangling FK errors.
This test relies on the fact that contenttypes is always loaded.
"""
new_apps = Apps()
class TestModel(models.Model):
ct = models.ForeignKey("contenttypes.ContentType")
class Meta:
app_label = "migrations"
apps = new_apps
# If we just stick it into an empty state it should fail
project_state = ProjectState()
project_state.add_model_state(ModelState.from_model(TestModel))
with self.assertRaises(ValueError):
rendered_state = project_state.render()
# If we include the real app it should succeed
project_state = ProjectState(real_apps=["contenttypes"])
project_state.add_model_state(ModelState.from_model(TestModel))
rendered_state = project_state.render()
self.assertEqual(len(rendered_state.get_models()), 2)
class ModelStateTests(TestCase): class ModelStateTests(TestCase):
def test_custom_model_base(self): def test_custom_model_base(self):