From 96b8b006363be122c5cfd2b54b2ef5c0c67a0315 Mon Sep 17 00:00:00 2001 From: Dylan Grafmyre Date: Mon, 6 Feb 2023 20:31:27 +0000 Subject: [PATCH] contrib-nested-django-apps, support migrations discovered within a submodule this feature requires an opt-in flag, named --resolve-nested-apps. enabling this flag will attempt to map migration files to django.apps.apps.app_configs by using both's relative path to self.django_path (or settings.BASE_DIR). if the app_config's relative path matches the migration files's parent folder's relative path, that is considered a match and the migration is associated with the app_config.label for later lookups. --- django_migration_linter/constants.py | 2 +- .../management/commands/lintmigrations.py | 13 +++ django_migration_linter/migration_linter.py | 100 ++++++++++++++---- django_migration_linter/utils.py | 15 +++ tests/test_project/app_nested/__init__.py | 0 .../app_nested/app_subapp/__init__.py | 0 .../app_nested/app_subapp/apps.py | 6 ++ .../app_subapp/migrations/0001_initial.py | 30 ++++++ .../app_subapp/migrations/0002_foo.py | 16 +++ .../app_subapp/migrations/__init__.py | 0 .../app_nested/app_subapp/models.py | 8 ++ tests/test_project/app_nested/apps.py | 6 ++ .../app_nested/migrations/0001_initial.py | 30 ++++++ .../app_nested/migrations/0002_foo.py | 16 +++ .../app_nested/migrations/__init__.py | 0 tests/test_project/app_nested/models.py | 8 ++ tests/test_project/settings.py | 2 + tests/unit/test_linter.py | 81 ++++++++++++++ tests/unit/test_utils.py | 20 +++- 19 files changed, 328 insertions(+), 25 deletions(-) create mode 100644 tests/test_project/app_nested/__init__.py create mode 100644 tests/test_project/app_nested/app_subapp/__init__.py create mode 100644 tests/test_project/app_nested/app_subapp/apps.py create mode 100644 tests/test_project/app_nested/app_subapp/migrations/0001_initial.py create mode 100644 tests/test_project/app_nested/app_subapp/migrations/0002_foo.py create mode 100644 tests/test_project/app_nested/app_subapp/migrations/__init__.py create mode 100644 tests/test_project/app_nested/app_subapp/models.py create mode 100644 tests/test_project/app_nested/apps.py create mode 100644 tests/test_project/app_nested/migrations/0001_initial.py create mode 100644 tests/test_project/app_nested/migrations/0002_foo.py create mode 100644 tests/test_project/app_nested/migrations/__init__.py create mode 100644 tests/test_project/app_nested/models.py diff --git a/django_migration_linter/constants.py b/django_migration_linter/constants.py index 3983c549..5702f662 100644 --- a/django_migration_linter/constants.py +++ b/django_migration_linter/constants.py @@ -2,7 +2,7 @@ from appdirs import user_cache_dir -__version__ = "4.1.0" +__version__ = "4.2.0" DEFAULT_CACHE_PATH = user_cache_dir("django-migration-linter", version=__version__) diff --git a/django_migration_linter/management/commands/lintmigrations.py b/django_migration_linter/management/commands/lintmigrations.py index 73170616..d40bcdef 100644 --- a/django_migration_linter/management/commands/lintmigrations.py +++ b/django_migration_linter/management/commands/lintmigrations.py @@ -87,6 +87,18 @@ def add_arguments(self, parser: CommandParser) -> None: nargs="?", help="if specified, only migrations listed in the file will be considered", ) + parser.add_argument( + "--resolve-nested-apps", + action="store_true", + help=( + "if specified, read_migrations_list migration files and" + " _gather_migrations_git file names will be indexed by their file" + " system prefix, relative to 'settings.BASE_DIR', which should" + " match the loaded django appconfig's path. on IndexError" + " falls-through to normal folder-named must be app_label based" + " app_config mapping." + ), + ) cache_group = parser.add_mutually_exclusive_group(required=False) cache_group.add_argument( "--cache-path", @@ -181,6 +193,7 @@ def handle(self, *args, **options): migration_name=options["migration_name"], git_commit_id=options["git_commit_id"], migrations_file_path=options["include_migrations_from"], + resolve_nested_apps=options["resolve_nested_apps"], ) linter.print_summary() if linter.has_errors: diff --git a/django_migration_linter/migration_linter.py b/django_migration_linter/migration_linter.py index f73636ad..cd762791 100644 --- a/django_migration_linter/migration_linter.py +++ b/django_migration_linter/migration_linter.py @@ -14,6 +14,7 @@ from django.db import DEFAULT_DB_ALIAS, ProgrammingError, connections from django.db.migrations import Migration, RunPython, RunSQL from django.db.migrations.operations.base import Operation +from django.apps import apps from .cache import Cache from .constants import ( @@ -24,7 +25,7 @@ from .operations import IgnoreMigration from .sql_analyser import analyse_sql_statements, get_sql_analyser_class from .sql_analyser.base import Issue -from .utils import clean_bytes_to_str, get_migration_abspath, split_migration_path +from .utils import clean_bytes_to_str, get_migration_abspath, split_migration_path, split_migration_prefix logger = logging.getLogger("django_migration_linter") @@ -41,6 +42,17 @@ def values() -> list[str]: return list(map(lambda c: c.value, MessageType)) + +_appconfigs_relpath_index = None +def appconfigs_relpath_index(): + global _appconfigs_relpath_index + if _appconfigs_relpath_index is None: + _appconfigs_relpath_index = { + os.path.relpath(v.path, settings.BASE_DIR): k + for k,v in apps.app_configs.items() + } + return _appconfigs_relpath_index + class MigrationLinter: def __init__( self, @@ -118,11 +130,12 @@ def lint_all_migrations( migration_name: str | None = None, git_commit_id: str | None = None, migrations_file_path: str | None = None, + resolve_nested_apps: bool | None = False, ) -> None: # Collect migrations. - migrations_list = self.read_migrations_list(migrations_file_path) + migrations_list = self.read_migrations_list(migrations_file_path, resolve_nested_apps) if git_commit_id: - migrations = self._gather_migrations_git(git_commit_id, migrations_list) + migrations = self._gather_migrations_git(git_commit_id, migrations_list, resolve_nested_apps) else: migrations = self._gather_all_migrations(migrations_list) @@ -338,9 +351,35 @@ def is_migration_file(filename: str) -> bool: and "__init__" not in filename ) + + def _resolve_nested_apps(migration_file: str) -> tuple[str, str] | None: + prefix, name = split_migration_prefix(migration_file) + if prefix in appconfigs_relpath_index(): + app_label = apps.app_configs[appconfigs_relpath_index()[prefix]].label + return (app_label, name, None) + + return (None, None, prefix) + + def _resolve_nested_apps_or_split_migration_path( + migration_file: str, + resolve_nested_apps: bool | None = False + ) -> tuple[str, str]: + if resolve_nested_apps: + app_label, name, prefix = MigrationLinter._resolve_nested_apps(migration_file) + if app_label is not None: + return (app_label, name) + + logger.warning( + "IndexError: key %r not in relative django.apps.apps.app_configs[].path list, assuming app_label is parent folder name of ./migrations", + prefix + ) + + return split_migration_path(migration_file) + @classmethod def read_migrations_list( - cls, migrations_file_path: str | None + cls, migrations_file_path: str | None, + resolve_nested_apps: bool | None = False ) -> list[tuple[str, str]] | None: """ Returning an empty list is different from returning None here. @@ -355,8 +394,9 @@ def read_migrations_list( with open(migrations_file_path) as file: for line in file: if cls.is_migration_file(line): - app_label, name = split_migration_path(line) - migrations.append((app_label, name)) + migrations.append( + cls._resolve_nested_apps_or_split_migration_path(line, resolve_nested_apps) + ) except OSError: logger.exception("Migrations list path not found %s", migrations_file_path) raise Exception("Error while reading migrations list file") @@ -368,8 +408,35 @@ def read_migrations_list( ) return migrations + def _gather_migrations_git__inner( + self, line: str, + migrations_list: list[tuple[str, str]] | None = None, + resolve_nested_apps: bool | None = False + ) -> tuple[str, str]: + # Only gather lines that include added migrations + if self.is_migration_file(line): + if resolve_nested_apps: + if not line.startswith(os.path.basename(self.django_path)): + line = os.path.join(os.path.basename(self.django_path), line) + app_label, name = MigrationLinter._resolve_nested_apps_or_split_migration_path(line, resolve_nested_apps) + if migrations_list is None or (app_label, name) in migrations_list: + if (app_label, name) in self.migration_loader.disk_migrations: + migration = self.migration_loader.disk_migrations[ + app_label, name + ] + return migration + else: + logger.info( + "Found migration file (%s, %s) " + "that is not present in loaded migration.", + app_label, + name, + ) + return None + def _gather_migrations_git( - self, git_commit_id: str, migrations_list: list[tuple[str, str]] | None = None + self, git_commit_id: str, migrations_list: list[tuple[str, str]] | None = None, + resolve_nested_apps: bool | None = False ) -> Iterable[Migration]: migrations = [] # Get changes since specified commit @@ -381,22 +448,9 @@ def _gather_migrations_git( for line in map( clean_bytes_to_str, diff_process.stdout.readlines() # type: ignore ): - # Only gather lines that include added migrations - if self.is_migration_file(line): - app_label, name = split_migration_path(line) - if migrations_list is None or (app_label, name) in migrations_list: - if (app_label, name) in self.migration_loader.disk_migrations: - migration = self.migration_loader.disk_migrations[ - app_label, name - ] - migrations.append(migration) - else: - logger.info( - "Found migration file (%s, %s) " - "that is not present in loaded migration.", - app_label, - name, - ) + migration = self._gather_migrations_git__inner(line, migrations_list, resolve_nested_apps) + if migration is not None: + migrations.append(migration) diff_process.wait() if diff_process.returncode != 0: diff --git a/django_migration_linter/utils.py b/django_migration_linter/utils.py index 8757e5bd..efcae8c2 100644 --- a/django_migration_linter/utils.py +++ b/django_migration_linter/utils.py @@ -23,6 +23,12 @@ def split_path(path: str) -> list[str]: return decomposed_path +def join_path(components: tuple[str, ...], sep:str=os.path.sep) -> str: + if components[0] == sep: + components[0] = '' + return sep.join(components) + + def split_migration_path(migration_path: str) -> tuple[str, str]: from django.db.migrations.loader import MIGRATIONS_MODULE_NAME @@ -32,6 +38,15 @@ def split_migration_path(migration_path: str) -> tuple[str, str]: return decomposed_path[i - 1], os.path.splitext(decomposed_path[i + 1])[0] return "", "" +def split_migration_prefix(migration_path: str) -> tuple[str, str]: + from django.db.migrations.loader import MIGRATIONS_MODULE_NAME + + decomposed_path = split_path(migration_path) + for i, p in enumerate(decomposed_path): + if p == MIGRATIONS_MODULE_NAME: + return join_path(decomposed_path[:i]), os.path.splitext(decomposed_path[i + 1])[0] + return "", "" + def clean_bytes_to_str(byte_input: bytes) -> str: return byte_input.decode("utf-8").strip() diff --git a/tests/test_project/app_nested/__init__.py b/tests/test_project/app_nested/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_project/app_nested/app_subapp/__init__.py b/tests/test_project/app_nested/app_subapp/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_project/app_nested/app_subapp/apps.py b/tests/test_project/app_nested/app_subapp/apps.py new file mode 100644 index 00000000..53bb0081 --- /dev/null +++ b/tests/test_project/app_nested/app_subapp/apps.py @@ -0,0 +1,6 @@ +from django.apps import AppConfig + + +class AppSubappConfig(AppConfig): + name = "tests.test_project.app_nested.app_subapp" + label = "app_nested_app_subapp" diff --git a/tests/test_project/app_nested/app_subapp/migrations/0001_initial.py b/tests/test_project/app_nested/app_subapp/migrations/0001_initial.py new file mode 100644 index 00000000..6edadcfe --- /dev/null +++ b/tests/test_project/app_nested/app_subapp/migrations/0001_initial.py @@ -0,0 +1,30 @@ +# Generated by Django 2.1.4 on 2019-03-19 21:08 + +from __future__ import annotations + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="A", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("null_field", models.IntegerField(null=True)), + ], + ) + ] diff --git a/tests/test_project/app_nested/app_subapp/migrations/0002_foo.py b/tests/test_project/app_nested/app_subapp/migrations/0002_foo.py new file mode 100644 index 00000000..1d3089b1 --- /dev/null +++ b/tests/test_project/app_nested/app_subapp/migrations/0002_foo.py @@ -0,0 +1,16 @@ +# Generated by Django 2.1.4 on 2019-03-19 21:08 + +from __future__ import annotations + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [("app_nested_app_subapp", "0001_initial")] + + operations = [ + migrations.AddField( + model_name="a", name="new_null_field", field=models.IntegerField(null=True) + ) + ] diff --git a/tests/test_project/app_nested/app_subapp/migrations/__init__.py b/tests/test_project/app_nested/app_subapp/migrations/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_project/app_nested/app_subapp/models.py b/tests/test_project/app_nested/app_subapp/models.py new file mode 100644 index 00000000..21f732fd --- /dev/null +++ b/tests/test_project/app_nested/app_subapp/models.py @@ -0,0 +1,8 @@ +from __future__ import annotations + +from django.db import models + + +class A(models.Model): + null_field = models.IntegerField(null=True) + new_null_field = models.IntegerField(null=True) diff --git a/tests/test_project/app_nested/apps.py b/tests/test_project/app_nested/apps.py new file mode 100644 index 00000000..f991cf55 --- /dev/null +++ b/tests/test_project/app_nested/apps.py @@ -0,0 +1,6 @@ +from django.apps import AppConfig + + +class AppNestedConfig(AppConfig): + name = "tests.test_project.app_nested" + label = "app_nested" diff --git a/tests/test_project/app_nested/migrations/0001_initial.py b/tests/test_project/app_nested/migrations/0001_initial.py new file mode 100644 index 00000000..6edadcfe --- /dev/null +++ b/tests/test_project/app_nested/migrations/0001_initial.py @@ -0,0 +1,30 @@ +# Generated by Django 2.1.4 on 2019-03-19 21:08 + +from __future__ import annotations + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="A", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("null_field", models.IntegerField(null=True)), + ], + ) + ] diff --git a/tests/test_project/app_nested/migrations/0002_foo.py b/tests/test_project/app_nested/migrations/0002_foo.py new file mode 100644 index 00000000..fca38db7 --- /dev/null +++ b/tests/test_project/app_nested/migrations/0002_foo.py @@ -0,0 +1,16 @@ +# Generated by Django 2.1.4 on 2019-03-19 21:08 + +from __future__ import annotations + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [("app_nested", "0001_initial")] + + operations = [ + migrations.AddField( + model_name="a", name="new_null_field", field=models.IntegerField(null=True) + ) + ] diff --git a/tests/test_project/app_nested/migrations/__init__.py b/tests/test_project/app_nested/migrations/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_project/app_nested/models.py b/tests/test_project/app_nested/models.py new file mode 100644 index 00000000..21f732fd --- /dev/null +++ b/tests/test_project/app_nested/models.py @@ -0,0 +1,8 @@ +from __future__ import annotations + +from django.db import models + + +class A(models.Model): + null_field = models.IntegerField(null=True) + new_null_field = models.IntegerField(null=True) diff --git a/tests/test_project/settings.py b/tests/test_project/settings.py index 271dc4c6..d67d0538 100644 --- a/tests/test_project/settings.py +++ b/tests/test_project/settings.py @@ -56,6 +56,8 @@ "tests.test_project.app_data_migrations", "tests.test_project.app_make_not_null_with_django_default", "tests.test_project.app_make_not_null_with_lib_default", + "tests.test_project.app_nested", + "tests.test_project.app_nested.app_subapp", ] MIDDLEWARE = [ diff --git a/tests/unit/test_linter.py b/tests/unit/test_linter.py index 8c2b57bd..8ac236b8 100644 --- a/tests/unit/test_linter.py +++ b/tests/unit/test_linter.py @@ -4,6 +4,8 @@ import unittest from django.db.migrations import Migration +from django.test import tag + from django_migration_linter import MigrationLinter @@ -74,6 +76,7 @@ def test_include_migration_multiple_names(self): self.assertFalse(linter.should_ignore_migration("app_correct", "0002_foo")) self.assertFalse(linter.should_ignore_migration("app_correct", "0003_bar")) + @tag('gather') def test_gather_all_migrations(self): linter = MigrationLinter() migrations = linter._gather_all_migrations() @@ -137,6 +140,45 @@ def test_read_migrations_from_file(self): migration_list, ) + def test_read_migrations_app_subapp_legacy(self): + tmp = tempfile.NamedTemporaryFile(mode="w", delete=False) + tmp.write( + "test_project/app_nested/app_subapp/migrations/0001_create_table.py\n" + ) + tmp.write("unknown\n") + tmp.write( + "test_project/app_nested/app_subapp/migrations/0002_add_new_not_null_field.py\n" + ) + tmp.close() + migration_list = MigrationLinter.read_migrations_list(tmp.name, resolve_nested_apps=False) + self.assertEqual( + [ + ("app_subapp", "0001_create_table"), + ("app_subapp", "0002_add_new_not_null_field"), + ], + migration_list, + ) + + def test_read_migrations_app_subapp(self): + tmp = tempfile.NamedTemporaryFile(mode="w", delete=False) + tmp.write( + "test_project/app_nested/app_subapp/migrations/0001_create_table.py\n" + ) + tmp.write("unknown\n") + tmp.write( + "test_project/app_nested/app_subapp/migrations/0002_add_new_not_null_field.py\n" + ) + tmp.close() + migration_list = MigrationLinter.read_migrations_list(tmp.name, resolve_nested_apps=True) + self.assertEqual( + [ + ("app_nested_app_subapp", "0001_create_table"), + ("app_nested_app_subapp", "0002_add_new_not_null_field"), + ], + migration_list, + ) + + @tag('gather') def test_gather_migrations_with_list(self): linter = MigrationLinter() migrations = linter._gather_all_migrations( @@ -146,3 +188,42 @@ def test_gather_migrations_with_list(self): ] ) self.assertEqual(2, len(list(migrations))) + + @tag('gather') + def test_gather_migration_git_correct(self): + linter = MigrationLinter("tests/test_project", only_applied_migrations=True) + migrations = [ + linter._gather_migrations_git__inner("app_correct/migrations/0001_initial.py"), + linter._gather_migrations_git__inner("app_correct/migrations/0002_foo.py"), + ] + self.assertEqual( + [ + (migrations[0].app_label, migrations[0].name), + (migrations[1].app_label, migrations[1].name), + ], + [ + ("app_correct", "0001_initial"), + ("app_correct", "0002_foo"), + ], + ) + + @tag('gather') + def test_gather_migration_git_nested(self): + linter = MigrationLinter("tests/test_project", only_applied_migrations=True) + migrations = [ + linter._gather_migrations_git__inner("app_nested/app_subapp/migrations/0001_initial.py", resolve_nested_apps=True), + linter._gather_migrations_git__inner("app_nested/app_subapp/migrations/0002_foo.py", resolve_nested_apps=True), + ] + + self.assertTrue(migrations[0]) + self.assertTrue(migrations[1]) + self.assertEqual( + [ + (migrations[0].app_label, migrations[0].name), + (migrations[1].app_label, migrations[1].name), + ], + [ + ("app_nested_app_subapp", "0001_initial"), + ("app_nested_app_subapp", "0002_foo"), + ], + ) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 733b5c8e..807fde44 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -2,7 +2,7 @@ import unittest -from django_migration_linter.utils import split_migration_path, split_path +from django_migration_linter.utils import split_migration_path, split_path, split_migration_prefix class SplitPathTestCase(unittest.TestCase): @@ -43,3 +43,21 @@ def test_split_migration_full_path(self): app, mig = split_migration_path(input_path) self.assertEqual(app, "the_app") self.assertEqual(mig, "0001_stuff") + + def test_split_migration_long_prefix(self): + input_path = "apps/the_app/migrations/0001_stuff.py" + prefix, mig = split_migration_prefix(input_path) + self.assertEqual(prefix, "apps/the_app") + self.assertEqual(mig, "0001_stuff") + + def test_split_migration_prefix(self): + input_path = "the_app/migrations/0001_stuff.py" + prefix, mig = split_migration_prefix(input_path) + self.assertEqual(prefix, "the_app") + self.assertEqual(mig, "0001_stuff") + + def test_split_migration_full_prefix(self): + input_path = "/home/user/djangostuff/apps/the_app/migrations/0001_stuff.py" + prefix, mig = split_migration_prefix(input_path) + self.assertEqual(prefix, "/home/user/djangostuff/apps/the_app") + self.assertEqual(mig, "0001_stuff")