Skip to content

Commit

Permalink
Build graph plan once for all app labels
Browse files Browse the repository at this point in the history
  • Loading branch information
q0w committed Oct 17, 2022
1 parent 4a73735 commit e6a5891
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 94 deletions.
39 changes: 31 additions & 8 deletions src/django_linear_migrations/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from django.apps import AppConfig, apps
from django.conf import settings
from django.core.checks import Error, Tags, register
from django.core.management import CommandError
from django.core.signals import setting_changed
from django.db import DEFAULT_DB_ALIAS, connections
from django.db.migrations.loader import MigrationLoader
Expand Down Expand Up @@ -54,6 +55,34 @@ def first_party_app_configs() -> Generator[AppConfig, None, None]:
yield app_config


def get_graph_plan(
app_names: Iterable[str] | None = None,
database: str = DEFAULT_DB_ALIAS,
) -> list[tuple[str, str]]:
loader = MigrationLoader(connections[database], ignore_no_migrations=True)
conflicts = loader.detect_conflicts()
if app_names:
conflicts = {
app_label: conflict
for app_label, conflict in conflicts.items()
if app_label in app_names
}
if conflicts:
name_str = "; ".join(
f"{', '.join(names)} in {app}" for app, names in conflicts.items()
)
raise CommandError(
"Conflicting migrations detected; multiple leaf nodes in the "
+ f"migration graph: {name_str}.\n"
+ "To fix them run 'python manage.py makemigrations --merge'"
)
nodes = loader.graph.leaf_nodes()
if app_names:
nodes = [key for key in loader.graph.leaf_nodes() if key[0] in app_names]
plan = loader.graph._generate_plan(nodes, at_end=True)
return cast(list[tuple[str, str]], plan)


class MigrationDetails:
migrations_module_name: str | None
migrations_module: ModuleType | None
Expand Down Expand Up @@ -107,13 +136,6 @@ def names(self) -> set[str]:
if not is_pkg and name[0] not in "_~"
}

@cached_property
def plan(self) -> list[tuple[str, str]]:
loader = MigrationLoader(connections[DEFAULT_DB_ALIAS])
nodes = [key for key in loader.graph.leaf_nodes() if key[0] in self.app_label]
plan = loader.graph._generate_plan(nodes, at_end=True)
return cast(list[tuple[str, str]], plan)


def check_max_migration_files(
*, app_configs: Iterable[AppConfig] | None = None, **kwargs: object
Expand All @@ -124,6 +146,7 @@ def check_max_migration_files(
else:
app_config_set = set()

graph_plan = get_graph_plan(app_names=[a.label for a in first_party_app_configs()])
for app_config in first_party_app_configs():
# When only checking certain apps, skip the others
if app_configs is not None and app_config not in app_config_set:
Expand Down Expand Up @@ -181,7 +204,7 @@ def check_max_migration_files(
)
continue

_, real_max_migration_name = migration_details.plan[-1]
real_max_migration_name = [k[1] for k in graph_plan if k[0] == app_label][-1]
if max_migration_name != real_max_migration_name:
errors.append(
Error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
from django.apps import apps
from django.core.management.commands.makemigrations import Command as BaseCommand

from django_linear_migrations.apps import MigrationDetails, first_party_app_configs
from django_linear_migrations.apps import (
MigrationDetails,
first_party_app_configs,
get_graph_plan,
)


class Command(BaseCommand):
Expand Down Expand Up @@ -56,6 +60,7 @@ def handle(
sys.exit(2)

any_created = False
graph_plan = get_graph_plan(app_names=labels)
for app_config in first_party_app_configs():
if labels and app_config.label not in labels:
continue
Expand All @@ -67,7 +72,9 @@ def handle(
max_migration_txt = migration_details.dir / "max_migration.txt"
if recreate or not max_migration_txt.exists():
if not dry_run:
_, max_migration_name = migration_details.plan[-1]
max_migration_name = [
k[1] for k in graph_plan if k[0] == app_config.label
][-1]
max_migration_txt.write_text(max_migration_name + "\n")
self.stdout.write(
f"Created max_migration.txt for {app_config.label}."
Expand Down
72 changes: 50 additions & 22 deletions tests/test_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
from textwrap import dedent

import pytest
from django.core.management import CommandError
from django.test import TestCase, override_settings

from django_linear_migrations.apps import check_max_migration_files
from tests.utils import empty_migration


class CheckMaxMigrationFilesTests(TestCase):
Expand Down Expand Up @@ -54,7 +56,7 @@ def test_skipped_unspecified_app(self):

def test_dlm_E001(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())

result = check_max_migration_files()

Expand All @@ -64,7 +66,7 @@ def test_dlm_E001(self):

def test_dlm_E002(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())
(self.migrations_dir / "max_migration.txt").write_text("line1\nline2\n")

result = check_max_migration_files()
Expand All @@ -75,7 +77,7 @@ def test_dlm_E002(self):

def test_dlm_E003(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())
(self.migrations_dir / "max_migration.txt").write_text("0001_start\n")

result = check_max_migration_files()
Expand All @@ -89,21 +91,13 @@ def test_dlm_E003(self):

def test_dlm_E004(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(
dedent(
"""
from django.db import migrations
class Migration(migrations.Migration):
pass
"""
)
)
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())
(self.migrations_dir / "0002_updates.py").write_text(
dedent(
"""
from django.db import migrations
class Migration(migrations.Migration):
pass
dependencies = [('testapp', '0001_initial')]
"""
)
)
Expand All @@ -118,17 +112,51 @@ class Migration(migrations.Migration):
+ " latest migration is '0002_updates'."
)

def test_okay(self):
migrations_txt = dedent(
"""
from django.db import migrations
class Migration(migrations.Migration):
pass
"""
def test_dlm_E005(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())
(self.migrations_dir / "custom_name.py").write_text(
dedent(
"""
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [('testapp', '0001_initial')]
"""
)
)
(self.migrations_dir / "0002_updates.py").write_text(
dedent(
"""
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [('testapp', '0001_initial')]
"""
)
)
(self.migrations_dir / "max_migration.txt").write_text("0002_updates\n")

with pytest.raises(CommandError):
result = check_max_migration_files()
assert len(result) == 1
assert result[0].id == "dlm.E005"
assert (
result[0].msg
== "Conflicting migrations detected; multiple leaf nodes"
+ " in the migration graph: 0002_updates, custom_name in testapp."
)

def test_okay(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(migrations_txt)
(self.migrations_dir / "0002_updates.py").write_text(migrations_txt)
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())
(self.migrations_dir / "0002_updates.py").write_text(
dedent(
"""
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [('testapp', '0001_initial')]
"""
)
)
(self.migrations_dir / "max_migration.txt").write_text("0002_updates\n")

result = check_max_migration_files()
Expand Down
50 changes: 10 additions & 40 deletions tests/test_create_max_migration_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
from django.core.management import call_command
from django.test import TestCase, override_settings

from tests.utils import empty_migration


class CreateMaxMigrationFilesTests(TestCase):
@pytest.fixture(autouse=True)
Expand Down Expand Up @@ -77,7 +79,7 @@ def test_success_only_init(self):
@override_settings(FIRST_PARTY_APPS=[])
def test_success_setting_not_first_party(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())

out, err, returncode = self.call_command()

Expand All @@ -87,7 +89,7 @@ def test_success_setting_not_first_party(self):

def test_success_dry_run(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())

out, err, returncode = self.call_command("--dry-run")

Expand All @@ -99,15 +101,7 @@ def test_success_dry_run(self):

def test_success(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(
dedent(
"""
from django.db import migrations
class Migration(migrations.Migration):
pass
"""
)
)
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())

out, err, returncode = self.call_command()

Expand All @@ -119,7 +113,7 @@ class Migration(migrations.Migration):

def test_success_already_exists(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())
(self.migrations_dir / "max_migration.txt").write_text("0001_initial\n")

out, err, returncode = self.call_command()
Expand All @@ -130,15 +124,7 @@ def test_success_already_exists(self):

def test_success_recreate(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(
dedent(
"""
from django.db import migrations
class Migration(migrations.Migration):
pass
"""
)
)
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())
(self.migrations_dir / "max_migration.txt").write_text("0001_initial\n")

out, err, returncode = self.call_command("--recreate")
Expand All @@ -149,7 +135,7 @@ class Migration(migrations.Migration):

def test_success_recreate_dry_run(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())
(self.migrations_dir / "max_migration.txt").write_text("0001_initial\n")

out, err, returncode = self.call_command("--recreate", "--dry-run")
Expand All @@ -160,15 +146,7 @@ def test_success_recreate_dry_run(self):

def test_success_specific_app_label(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(
dedent(
"""
from django.db import migrations
class Migration(migrations.Migration):
pass
"""
)
)
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())

out, err, returncode = self.call_command("testapp")

Expand Down Expand Up @@ -196,15 +174,7 @@ def test_success_ignored_app_label(self):

def test_success_custom_migration_name(self):
(self.migrations_dir / "__init__.py").touch()
(self.migrations_dir / "0001_initial.py").write_text(
dedent(
"""
from django.db import migrations
class Migration(migrations.Migration):
pass
"""
)
)
(self.migrations_dir / "0001_initial.py").write_text(empty_migration())
(self.migrations_dir / "custom_name.py").write_text(
dedent(
"""
Expand Down
Loading

0 comments on commit e6a5891

Please sign in to comment.