diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/__init__.py b/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/0001_initial.py b/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/0001_initial.py new file mode 100644 index 00000000000000..69de29635772a4 --- /dev/null +++ b/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/0001_initial.py @@ -0,0 +1,26 @@ +# Generated by Django 3.1 on 2019-09-22 21:47 + +from django.db import migrations, models + +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="TestTable", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ], + ), + ] diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/0002_delete_pending.py b/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/0002_delete_pending.py new file mode 100644 index 00000000000000..917b83001e8a8d --- /dev/null +++ b/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/0002_delete_pending.py @@ -0,0 +1,17 @@ +from sentry.new_migrations.migrations import CheckedMigration +from sentry.new_migrations.monkey.models import SafeDeleteModel +from sentry.new_migrations.monkey.state import DeletionAction + + +class Migration(CheckedMigration): + + dependencies = [ + ("bad_flow_delete_model_double_pending_app", "0001_initial"), + ] + + operations = [ + SafeDeleteModel( + name="TestTable", + deletion_action=DeletionAction.MOVE_TO_PENDING, + ), + ] diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/0003_double_pending.py b/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/0003_double_pending.py new file mode 100644 index 00000000000000..e52a64272a2020 --- /dev/null +++ b/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/0003_double_pending.py @@ -0,0 +1,17 @@ +from sentry.new_migrations.migrations import CheckedMigration +from sentry.new_migrations.monkey.models import SafeDeleteModel +from sentry.new_migrations.monkey.state import DeletionAction + + +class Migration(CheckedMigration): + + dependencies = [ + ("bad_flow_delete_model_double_pending_app", "0002_delete_pending"), + ] + + operations = [ + SafeDeleteModel( + name="TestTable", + deletion_action=DeletionAction.MOVE_TO_PENDING, + ), + ] diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/__init__.py b/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/models.py b/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/models.py new file mode 100644 index 00000000000000..770fa149c355ce --- /dev/null +++ b/fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/models.py @@ -0,0 +1,5 @@ +from django.db import models + + +class TestTable(models.Model): + field = models.IntegerField(default=0, null=False) diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/__init__.py b/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/migrations/0001_initial.py b/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/migrations/0001_initial.py new file mode 100644 index 00000000000000..69de29635772a4 --- /dev/null +++ b/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/migrations/0001_initial.py @@ -0,0 +1,26 @@ +# Generated by Django 3.1 on 2019-09-22 21:47 + +from django.db import migrations, models + +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="TestTable", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ], + ), + ] diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/migrations/0002_delete_without_pending.py b/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/migrations/0002_delete_without_pending.py new file mode 100644 index 00000000000000..10d83c4d9c3e39 --- /dev/null +++ b/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/migrations/0002_delete_without_pending.py @@ -0,0 +1,17 @@ +from sentry.new_migrations.migrations import CheckedMigration +from sentry.new_migrations.monkey.models import SafeDeleteModel +from sentry.new_migrations.monkey.state import DeletionAction + + +class Migration(CheckedMigration): + + dependencies = [ + ("bad_flow_delete_model_without_pending_app", "0001_initial"), + ] + + operations = [ + SafeDeleteModel( + name="TestTable", + deletion_action=DeletionAction.DELETE, + ), + ] diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/migrations/__init__.py b/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/migrations/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/models.py b/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/models.py new file mode 100644 index 00000000000000..770fa149c355ce --- /dev/null +++ b/fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/models.py @@ -0,0 +1,5 @@ +from django.db import models + + +class TestTable(models.Model): + field = models.IntegerField(default=0, null=False) diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/__init__.py b/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/migrations/0001_initial.py b/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/migrations/0001_initial.py new file mode 100644 index 00000000000000..2747f841d8a570 --- /dev/null +++ b/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/migrations/0001_initial.py @@ -0,0 +1,45 @@ +import django +from django.db import migrations, models + +import sentry +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="FkTable", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ], + ), + migrations.CreateModel( + name="TestTable", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ( + "fk_table", + sentry.db.models.fields.foreignkey.FlexibleForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="bad_flow_delete_pending_with_fk_constraints_app.fktable", + db_index=False, + ), + ), + ], + ), + ] diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/migrations/0002_delete_without_pending.py b/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/migrations/0002_delete_without_pending.py new file mode 100644 index 00000000000000..721c22b2798a0e --- /dev/null +++ b/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/migrations/0002_delete_without_pending.py @@ -0,0 +1,18 @@ +from sentry.new_migrations.migrations import CheckedMigration +from sentry.new_migrations.monkey.models import SafeDeleteModel +from sentry.new_migrations.monkey.state import DeletionAction + + +class Migration(CheckedMigration): + atomic = False + + dependencies = [ + ("bad_flow_delete_pending_with_fk_constraints_app", "0001_initial"), + ] + + operations = [ + SafeDeleteModel( + name="TestTable", + deletion_action=DeletionAction.MOVE_TO_PENDING, + ), + ] diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/migrations/__init__.py b/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/migrations/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/models.py b/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/models.py new file mode 100644 index 00000000000000..d936d5039213f7 --- /dev/null +++ b/fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/models.py @@ -0,0 +1,12 @@ +from django.db import models + +from sentry.db.models import FlexibleForeignKey + + +class FkTable(models.Model): + field = models.IntegerField(default=0, null=False) + + +class TestTable(models.Model): + field = models.IntegerField(default=0, null=False) + fk_table = FlexibleForeignKey(FkTable, db_index=False) diff --git a/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/__init__.py b/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/0001_initial.py b/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/0001_initial.py new file mode 100644 index 00000000000000..6f55cccc2e683d --- /dev/null +++ b/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/0001_initial.py @@ -0,0 +1,46 @@ +# Generated by Django 3.1 on 2019-09-22 21:47 +import django +from django.db import migrations, models + +import sentry +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="FkTable", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ], + ), + migrations.CreateModel( + name="TestTable", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ( + "fk_table", + sentry.db.models.fields.foreignkey.FlexibleForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="good_flow_delete_pending_with_fk_constraints_app.fktable", + db_index=False, + ), + ), + ], + ), + ] diff --git a/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/0002_remove_constraints_and_pending.py b/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/0002_remove_constraints_and_pending.py new file mode 100644 index 00000000000000..cdb0408e096d99 --- /dev/null +++ b/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/0002_remove_constraints_and_pending.py @@ -0,0 +1,32 @@ +import django +from django.db import migrations + +import sentry +from sentry.new_migrations.migrations import CheckedMigration +from sentry.new_migrations.monkey.models import SafeDeleteModel +from sentry.new_migrations.monkey.state import DeletionAction + + +class Migration(CheckedMigration): + atomic = False + + dependencies = [ + ("good_flow_delete_pending_with_fk_constraints_app", "0001_initial"), + ] + + operations = [ + migrations.AlterField( + model_name="TestTable", + name="fk_table", + field=sentry.db.models.fields.foreignkey.FlexibleForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="good_flow_delete_pending_with_fk_constraints_app.fktable", + db_index=False, + db_constraint=False, + ), + ), + SafeDeleteModel( + name="TestTable", + deletion_action=DeletionAction.MOVE_TO_PENDING, + ), + ] diff --git a/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/0003_delete.py b/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/0003_delete.py new file mode 100644 index 00000000000000..ac2813a8d7f014 --- /dev/null +++ b/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/0003_delete.py @@ -0,0 +1,16 @@ +from sentry.new_migrations.migrations import CheckedMigration +from sentry.new_migrations.monkey.models import SafeDeleteModel +from sentry.new_migrations.monkey.state import DeletionAction + + +class Migration(CheckedMigration): + dependencies = [ + ("good_flow_delete_pending_with_fk_constraints_app", "0002_remove_constraints_and_pending"), + ] + + operations = [ + SafeDeleteModel( + name="TestTable", + deletion_action=DeletionAction.DELETE, + ), + ] diff --git a/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/__init__.py b/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/models.py b/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/models.py new file mode 100644 index 00000000000000..d936d5039213f7 --- /dev/null +++ b/fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/models.py @@ -0,0 +1,12 @@ +from django.db import models + +from sentry.db.models import FlexibleForeignKey + + +class FkTable(models.Model): + field = models.IntegerField(default=0, null=False) + + +class TestTable(models.Model): + field = models.IntegerField(default=0, null=False) + fk_table = FlexibleForeignKey(FkTable, db_index=False) diff --git a/fixtures/safe_migrations_apps/good_flow_delete_simple_app/__init__.py b/fixtures/safe_migrations_apps/good_flow_delete_simple_app/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/fixtures/safe_migrations_apps/good_flow_delete_simple_app/migrations/0001_initial.py b/fixtures/safe_migrations_apps/good_flow_delete_simple_app/migrations/0001_initial.py new file mode 100644 index 00000000000000..2b6d293ee049e6 --- /dev/null +++ b/fixtures/safe_migrations_apps/good_flow_delete_simple_app/migrations/0001_initial.py @@ -0,0 +1,22 @@ +from django.db import migrations, models + +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + initial = True + dependencies = [] + + operations = [ + migrations.CreateModel( + name="TestTable", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ], + ), + ] diff --git a/fixtures/safe_migrations_apps/good_flow_delete_simple_app/migrations/0002_set_pending.py b/fixtures/safe_migrations_apps/good_flow_delete_simple_app/migrations/0002_set_pending.py new file mode 100644 index 00000000000000..c7475e451e0bcd --- /dev/null +++ b/fixtures/safe_migrations_apps/good_flow_delete_simple_app/migrations/0002_set_pending.py @@ -0,0 +1,16 @@ +from sentry.new_migrations.migrations import CheckedMigration +from sentry.new_migrations.monkey.models import SafeDeleteModel +from sentry.new_migrations.monkey.state import DeletionAction + + +class Migration(CheckedMigration): + dependencies = [ + ("good_flow_delete_simple_app", "0001_initial"), + ] + + operations = [ + SafeDeleteModel( + name="TestTable", + deletion_action=DeletionAction.MOVE_TO_PENDING, + ), + ] diff --git a/fixtures/safe_migrations_apps/good_flow_delete_simple_app/migrations/0003_delete.py b/fixtures/safe_migrations_apps/good_flow_delete_simple_app/migrations/0003_delete.py new file mode 100644 index 00000000000000..796cf774758675 --- /dev/null +++ b/fixtures/safe_migrations_apps/good_flow_delete_simple_app/migrations/0003_delete.py @@ -0,0 +1,16 @@ +from sentry.new_migrations.migrations import CheckedMigration +from sentry.new_migrations.monkey.models import SafeDeleteModel +from sentry.new_migrations.monkey.state import DeletionAction + + +class Migration(CheckedMigration): + dependencies = [ + ("good_flow_delete_simple_app", "0002_set_pending"), + ] + + operations = [ + SafeDeleteModel( + name="TestTable", + deletion_action=DeletionAction.DELETE, + ), + ] diff --git a/fixtures/safe_migrations_apps/good_flow_delete_simple_app/migrations/__init__.py b/fixtures/safe_migrations_apps/good_flow_delete_simple_app/migrations/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/fixtures/safe_migrations_apps/good_flow_delete_simple_app/models.py b/fixtures/safe_migrations_apps/good_flow_delete_simple_app/models.py new file mode 100644 index 00000000000000..770fa149c355ce --- /dev/null +++ b/fixtures/safe_migrations_apps/good_flow_delete_simple_app/models.py @@ -0,0 +1,5 @@ +from django.db import models + + +class TestTable(models.Model): + field = models.IntegerField(default=0, null=False) diff --git a/src/sentry/db/postgres/schema.py b/src/sentry/db/postgres/schema.py index e38c153cd77c70..986396c22e7122 100644 --- a/src/sentry/db/postgres/schema.py +++ b/src/sentry/db/postgres/schema.py @@ -88,14 +88,16 @@ def alter_db_table(self, model, old_db_table, new_db_table): "More info here: https://develop.sentry.dev/database-migrations/#renaming-tables" ) - def delete_model(self, model): + def delete_model(self, model, is_safe=False): """ It's never safe to delete a model using the standard migration process """ - raise UnsafeOperationException( - f"Deleting the {model.__name__} model is unsafe.\n" - "More info here: https://develop.sentry.dev/database-migrations/#deleting-tables" - ) + if not is_safe: + raise UnsafeOperationException( + f"Deleting the {model.__name__} model is unsafe.\n" + "More info here: https://develop.sentry.dev/database-migrations/#deleting-tables" + ) + super(DatabaseSchemaEditorMixin, self).delete_model(model) def remove_field(self, model, field): """ diff --git a/src/sentry/new_migrations/monkey/__init__.py b/src/sentry/new_migrations/monkey/__init__.py index a6b52294b510c1..d13eec497e52b1 100644 --- a/src/sentry/new_migrations/monkey/__init__.py +++ b/src/sentry/new_migrations/monkey/__init__.py @@ -19,6 +19,10 @@ is copied and modified from `Queryset.update()` to add `RETURNING ` to the update query. Verify that the `update` code hasn't significantly changed, and if it has update as needed. + - We monkeypatch `SentryProjectState` over `ProjectState` in a few places. Check where + Django is importing it and make sure that we're still patching correctly. + We also need to verify that the patched `SentryProjectState` isn't missing new + features added by Django. When you're happy that these changes are good to go, update `LAST_VERIFIED_DJANGO_VERSION` to the version of Django you're upgrading to. If the @@ -86,3 +90,11 @@ def monkey_migrations(): migration.Migration.initial = None writer.MIGRATION_TEMPLATE = SENTRY_MIGRATION_TEMPLATE models.Field.deconstruct = deconstruct # type: ignore[method-assign] + + from django.db.migrations import graph, state + + from sentry.new_migrations.monkey.state import SentryProjectState + + state.ProjectState = SentryProjectState # type: ignore[misc] + graph.ProjectState = SentryProjectState # type: ignore[attr-defined] + executor.ProjectState = SentryProjectState # type: ignore[attr-defined] diff --git a/src/sentry/new_migrations/monkey/models.py b/src/sentry/new_migrations/monkey/models.py new file mode 100644 index 00000000000000..e744e11b356e70 --- /dev/null +++ b/src/sentry/new_migrations/monkey/models.py @@ -0,0 +1,58 @@ +from django.db.migrations import DeleteModel +from django_zero_downtime_migrations.backends.postgres.schema import UnsafeOperationException + +from sentry.db.postgres.schema import SafePostgresDatabaseSchemaEditor +from sentry.new_migrations.monkey.state import DeletionAction, SentryProjectState + + +class SafeDeleteModel(DeleteModel): + def __init__(self, *args, deletion_action: DeletionAction, **kwargs): + super().__init__(*args, **kwargs) + self.deletion_action = deletion_action + + def state_forwards(self, app_label: str, state: SentryProjectState) -> None: # type: ignore[override] + if self.deletion_action == DeletionAction.MOVE_TO_PENDING: + model = state.apps.get_model(app_label, self.name) + fields_with_constraints = [ + f.name for f in model._meta.fields if getattr(f, "db_constraint", False) + ] + if fields_with_constraints: + raise UnsafeOperationException( + "Foreign key db constraints must be removed before dropping " + f"{app_label}.{self.name}. Fields with constraints: {fields_with_constraints}" + "More info: https://develop.sentry.dev/api-server/application-domains/database-migrations/#deleting-tables" + ) + state.remove_model(app_label, self.name_lower, deletion_action=self.deletion_action) + + def database_forwards( + self, + app_label: str, + schema_editor: SafePostgresDatabaseSchemaEditor, # type: ignore[override] + from_state: SentryProjectState, # type: ignore[override] + to_state: SentryProjectState, # type: ignore[override] + ) -> None: + if self.deletion_action == DeletionAction.MOVE_TO_PENDING: + return + + model = from_state.get_pending_deletion_model(app_label, self.name) + if self.allow_migrate_model(schema_editor.connection.alias, model): + schema_editor.delete_model(model, is_safe=True) + + def database_backwards( + self, + app_label: str, + schema_editor: SafePostgresDatabaseSchemaEditor, # type: ignore[override] + from_state: SentryProjectState, # type: ignore[override] + to_state: SentryProjectState, # type: ignore[override] + ) -> None: + if self.deletion_action == DeletionAction.MOVE_TO_PENDING: + return + model = to_state.get_pending_deletion_model(app_label, self.name) + if self.allow_migrate_model(schema_editor.connection.alias, model): + schema_editor.create_model(model) + + def describe(self) -> str: + if self.deletion_action == DeletionAction.MOVE_TO_PENDING: + return f"Moved model {self.name} to pending deletion state" + else: + return super().describe() diff --git a/src/sentry/new_migrations/monkey/state.py b/src/sentry/new_migrations/monkey/state.py new file mode 100644 index 00000000000000..a204ec81d13970 --- /dev/null +++ b/src/sentry/new_migrations/monkey/state.py @@ -0,0 +1,54 @@ +from __future__ import annotations + +from copy import deepcopy +from enum import Enum + +from django.db.migrations.state import ProjectState +from django.db.models import Model +from django_zero_downtime_migrations.backends.postgres.schema import UnsafeOperationException + + +class DeletionAction(Enum): + MOVE_TO_PENDING = 0 + DELETE = 1 + + +class SentryProjectState(ProjectState): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.pending_deletion_models: dict[tuple[str, str], type[Model]] = {} + + def get_pending_deletion_model(self, app_label: str, model_name: str) -> type[Model]: + model_key = (app_label.lower(), model_name.lower()) + if model_key not in self.pending_deletion_models: + raise UnsafeOperationException( + "Model must be in the pending deletion state before full deletion. " + "More info: https://develop.sentry.dev/api-server/application-domains/database-migrations/#deleting-tables" + ) + return self.pending_deletion_models[model_key] + + def remove_model( + self, app_label: str, model_name: str, deletion_action: DeletionAction | None = None + ) -> None: + model_key = (app_label.lower(), model_name.lower()) + if deletion_action == DeletionAction.DELETE: + if model_key not in self.pending_deletion_models: + raise UnsafeOperationException( + "Model must be in the pending deletion state before full deletion. " + "More info: https://develop.sentry.dev/api-server/application-domains/database-migrations/#deleting-tables" + ) + del self.pending_deletion_models[model_key] + return + if deletion_action == DeletionAction.MOVE_TO_PENDING: + if model_key in self.pending_deletion_models: + raise UnsafeOperationException( + f"{app_label}.{model_name} is already pending deletion. Use DeletionAction.DELETE to delete" + "More info: https://develop.sentry.dev/api-server/application-domains/database-migrations/#deleting-tables" + ) + self.pending_deletion_models[model_key] = self.apps.get_model(app_label, model_name) + super().remove_model(app_label, model_name) + + def clone(self) -> SentryProjectState: + new_state = super().clone() + new_state.pending_deletion_models = deepcopy(self.pending_deletion_models) # type: ignore[attr-defined] + return new_state # type: ignore[return-value] diff --git a/tests/sentry/db/postgres/schema/safe_migrations/integration/test_migrations.py b/tests/sentry/db/postgres/schema/safe_migrations/integration/test_migrations.py index 955f10c09a1615..0638959b6b9bca 100644 --- a/tests/sentry/db/postgres/schema/safe_migrations/integration/test_migrations.py +++ b/tests/sentry/db/postgres/schema/safe_migrations/integration/test_migrations.py @@ -37,22 +37,24 @@ def run_migration(self): self._run_migration(self.app, self.migrate_from) self._run_migration(self.app, self.migrate_to) - def _run_migration(self, app, migration): + def _run_migration(self, app, migration_name): with override_settings( INSTALLED_APPS=(f"{self.BASE_PATH}.{self.app}",), MIGRATION_MODULES={} ): executor = MigrationExecutor(connection) + migration = executor.loader.get_migration_by_prefix(app, migration_name) executor.loader.build_graph() - target = [(app, migration)] + target = [(migration.app_label, migration.name)] executor.loader.project_state(target).apps executor.migrate(target) - def sql_migrate(self, app_label, migration_name): + def sql_migrate(self, app, migration_name): with override_settings( INSTALLED_APPS=(f"{self.BASE_PATH}.{self.app}",), MIGRATION_MODULES={} ): - target = (app_label, migration_name) executor = MigrationExecutor(connection) + migration = executor.loader.get_migration_by_prefix(app, migration_name) + target = (app, migration.name) plan = [(executor.loader.graph.nodes[target], None)] sql_statements = executor.loader.collect_sql(plan) # type: ignore[attr-defined] return "\n".join(sql_statements) @@ -230,3 +232,73 @@ def test(self): "SET statement_timeout TO '0ms';", "SET lock_timeout TO '0ms';", ] + + +class DeletionBadDeleteModelWithoutPendingTest(BaseSafeMigrationTest): + app = "bad_flow_delete_model_without_pending_app" + migrate_from = "0001" + migrate_to = "0002" + + def test(self): + with pytest.raises( + UnsafeOperationException, + match="Model must be in the pending deletion state before full deletion", + ): + self.run_migration() + + +class DeletionBadDeleteModelDoublePendingTest(BaseSafeMigrationTest): + app = "bad_flow_delete_model_double_pending_app" + migrate_from = "0001" + migrate_to = "0003" + + def test(self): + with pytest.raises( + LookupError, + match="App 'bad_flow_delete_model_double_pending_app' doesn't have a 'TestTable' model", + ): + self.run_migration() + + +class DeletionBadDeletePendingWithFKConstraints(BaseSafeMigrationTest): + app = "bad_flow_delete_pending_with_fk_constraints_app" + migrate_from = "0001" + migrate_to = "0002" + + def test(self): + with pytest.raises( + UnsafeOperationException, + match="Foreign key db constraints must be removed before dropping " + "bad_flow_delete_pending_with_fk_constraints_app.TestTable. " + "Fields with constraints: \\['fk_table'\\]", + ): + self.run_migration() + + +class DeletionGoodDeleteRemoveFKConstraints(BaseSafeMigrationTest): + app = "good_flow_delete_pending_with_fk_constraints_app" + migrate_from = "0001" + migrate_to = "0003" + + def test(self): + + self._run_migration(self.app, "0001_initial") + assert f"{self.app}_testtable" in connection.introspection.table_names() + self._run_migration(self.app, "0002_remove_constraints_and_pending") + assert f"{self.app}_testtable" in connection.introspection.table_names() + self._run_migration(self.app, "0003_delete") + assert f"{self.app}_testtable" not in connection.introspection.table_names() + + +class DeletionGoodDeleteSimple(BaseSafeMigrationTest): + app = "good_flow_delete_simple_app" + migrate_from = "0001" + migrate_to = "0003" + + def test(self): + self._run_migration(self.app, "0001_initial") + assert f"{self.app}_testtable" in connection.introspection.table_names() + self._run_migration(self.app, "0002_set_pending") + assert f"{self.app}_testtable" in connection.introspection.table_names() + self._run_migration(self.app, "0003_delete") + assert f"{self.app}_testtable" not in connection.introspection.table_names()