fix: Long DELETE project call (#3360)

Co-authored-by: Matthew Elwell <matthew.elwell@flagsmith.com>

api/core/signals.py

@@ -30,6 +30,14 @@ def create_audit_log_from_historical_record(
         else None
     )
 
+    environment, project = history_instance.instance.get_environment_and_project()
+    if project != history_instance.instance and (
+        (project and project.deleted_at)
+        or (environment and environment.project.deleted_at)
+    ):
+        # don't trigger audit log records in deleted projects
+        return
+
     tasks.create_audit_log_from_historical_record.delay(
         kwargs={
             "history_instance_id": history_instance.history_id,

api/environments/migrations/0034_alter_environment_project.py

@@ -0,0 +1,20 @@
+# Generated by Django 3.2.23 on 2024-01-31 18:47
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('projects', '0021_add_identity_overrides_migration_status'),
+        ('environments', '0033_add_environment_feature_state_version_logic'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='environment',
+            name='project',
+            field=models.ForeignKey(help_text='Changing the project selected will remove all previous Feature States for the previously associated projects Features that are related to this Environment. New default Feature States will be created for the new selected projects Features for this Environment.', on_delete=django.db.models.deletion.DO_NOTHING, related_name='environments', to='projects.project'),
+        ),
+    ]

api/environments/models.py

@@ -86,7 +86,9 @@ class Environment(
             " Environment. New default Feature States will be created for the new"
             " selected projects Features for this Environment."
         ),
-        on_delete=models.CASCADE,
+        # Cascade deletes are decouple from the Django ORM. See this PR for details.
+        # https://github.com/Flagsmith/flagsmith/pull/3360/
+        on_delete=models.DO_NOTHING,
     )
 
     api_key = models.CharField(

api/environments/tasks.py

@@ -51,3 +51,8 @@ def delete_environment_from_dynamo(api_key: str, environment_id: str):
 
     # Delete environment_v2 documents
     environment_v2_wrapper.delete_environment(environment_id)
+
+
+@register_task_handler()
+def delete_environment(environment_id: int) -> None:
+    Environment.objects.get(id=environment_id).delete()

api/features/apps.py

@@ -12,3 +12,4 @@ def ready(self):
 
         # noinspection PyUnresolvedReferences
         import features.signals  # noqa
+        import features.tasks  # noqa

api/features/migrations/0063_detach_feature_from_project_cascade_delete.py

@@ -0,0 +1,20 @@
+# Generated by Django 3.2.23 on 2024-02-01 12:12
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('projects', '0021_add_identity_overrides_migration_status'),
+        ('features', '0062_alter_feature_segment_unique_together'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='feature',
+            name='project',
+            field=models.ForeignKey(help_text='Changing the project selected will remove previous Feature States for the previouslyassociated projects Environments that are related to this Feature. New default Feature States will be created for the new selected projects Environments for this Feature. Also this will remove any Tags associated with a feature as Tags are Project defined', on_delete=django.db.models.deletion.DO_NOTHING, related_name='features', to='projects.project'),
+        ),
+    ]

api/features/migrations/0064_fix_feature_help_text_typo.py

@@ -0,0 +1,25 @@
+# Generated by Django 3.2.23 on 2024-02-02 10:10
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('projects', '0021_add_identity_overrides_migration_status'),
+        ('features', '0063_detach_feature_from_project_cascade_delete'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='feature',
+            name='project',
+            field=models.ForeignKey(help_text='Changing the project selected will remove previous Feature States for the previously associated projects Environments that are related to this Feature. New default Feature States will be created for the new selected projects Environments for this Feature. Also this will remove any Tags associated with a feature as Tags are Project defined', on_delete=django.db.models.deletion.DO_NOTHING, related_name='features', to='projects.project'),
+        ),
+        migrations.AlterField(
+            model_name='historicalfeature',
+            name='project',
+            field=models.ForeignKey(blank=True, db_constraint=False, help_text='Changing the project selected will remove previous Feature States for the previously associated projects Environments that are related to this Feature. New default Feature States will be created for the new selected projects Environments for this Feature. Also this will remove any Tags associated with a feature as Tags are Project defined', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='projects.project'),
+        ),
+    ]

api/features/models.py

@@ -95,12 +95,14 @@ class Feature(
         Project,
         related_name="features",
         help_text=_(
-            "Changing the project selected will remove previous Feature States for the previously"
+            "Changing the project selected will remove previous Feature States for the previously "
             "associated projects Environments that are related to this Feature. New default "
             "Feature States will be created for the new selected projects Environments for this "
             "Feature. Also this will remove any Tags associated with a feature as Tags are Project defined"
         ),
-        on_delete=models.CASCADE,
+        # Cascade deletes are decouple from the Django ORM. See this PR for details.
+        # https://github.com/Flagsmith/flagsmith/pull/3360/
+        on_delete=models.DO_NOTHING,
     )
     initial_value = models.CharField(
         max_length=20000, null=True, default=None, blank=True

api/features/tasks.py

@@ -1,5 +1,6 @@
 from environments.models import Webhook
-from features.models import FeatureState
+from features.models import Feature, FeatureState
+from task_processor.decorators import register_task_handler
 from webhooks.constants import WEBHOOK_DATETIME_FORMAT
 from webhooks.webhooks import (
     WebhookEventType,
@@ -79,3 +80,8 @@ def _get_feature_state_webhook_data(feature_state, previous=False):
         identity_identifier=getattr(feature_state.identity, "identifier", None),
         feature_segment=feature_state.feature_segment,
     )
+
+
+@register_task_handler()
+def delete_feature(feature_id: int) -> None:
+    Feature.objects.get(pk=feature_id).delete()

api/projects/models.py

@@ -27,6 +27,7 @@
 )
 from projects.managers import ProjectManager
 from projects.tasks import (
+    handle_cascade_delete,
     migrate_project_environments_to_v2,
     write_environments_to_dynamodb,
 )
@@ -158,6 +159,10 @@ def write_to_dynamo(self):
     def clean_up_dynamo(self):
         DynamoProjectMetadata(self.id).delete()
 
+    @hook(AFTER_DELETE)
+    def handle_cascade_delete(self) -> None:
+        handle_cascade_delete.delay(kwargs={"project_id": self.id})
+
     @property
     def is_edge_project_by_default(self) -> bool:
         return bool(

api/projects/tasks.py

@@ -22,3 +22,29 @@ def migrate_project_environments_to_v2(project_id: int) -> None:
                 IdentityOverridesV2MigrationStatus.COMPLETE
             )
             project.save()
+
+
+@register_task_handler()
+def handle_cascade_delete(project_id: int) -> None:
+    """
+    Due to the combination of soft deletes and audit log functionality,
+    cascade deletes for a project can take a long time for large projects.
+    This task delegates the cascade delete to tasks for the main related
+    entities for a project.
+    """
+
+    from environments.tasks import delete_environment
+    from features.tasks import delete_feature
+    from projects.models import Project
+    from segments.tasks import delete_segment
+
+    project = Project.objects.all_with_deleted().get(id=project_id)
+
+    for environment_id in project.environments.values_list("id", flat=True):
+        delete_environment.delay(kwargs={"environment_id": environment_id})
+
+    for segment_id in project.segments.values_list("id", flat=True):
+        delete_segment.delay(kwargs={"segment_id": segment_id})
+
+    for feature_id in project.features.values_list("id", flat=True):
+        delete_feature.delay(kwargs={"feature_id": feature_id})

api/segments/apps.py

@@ -3,3 +3,8 @@
 
 class SegmentsConfig(BaseAppConfig):
     name = "segments"
+
+    def ready(self) -> None:
+        super().ready()
+
+        import segments.tasks  # noqa

api/segments/migrations/0020_detach_segment_from_project_cascade_delete.py

@@ -0,0 +1,20 @@
+# Generated by Django 3.2.23 on 2024-02-01 13:45
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('projects', '0021_add_identity_overrides_migration_status'),
+        ('segments', '0019_add_audit_to_condition'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='segment',
+            name='project',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.DO_NOTHING, related_name='segments', to='projects.project'),
+        ),
+    ]

api/segments/models.py

@@ -30,7 +30,11 @@ class Segment(
     name = models.CharField(max_length=2000)
     description = models.TextField(null=True, blank=True)
     project = models.ForeignKey(
-        Project, on_delete=models.CASCADE, related_name="segments"
+        Project,
+        # Cascade deletes are decouple from the Django ORM. See this PR for details.
+        # https://github.com/Flagsmith/flagsmith/pull/3360/
+        on_delete=models.DO_NOTHING,
+        related_name="segments",
     )
     feature = models.ForeignKey(
         Feature, on_delete=models.CASCADE, related_name="segments", null=True

api/segments/tasks.py

@@ -0,0 +1,7 @@
+from segments.models import Segment
+from task_processor.decorators import register_task_handler
+
+
+@register_task_handler()
+def delete_segment(segment_id: int) -> None:
+    Segment.objects.get(pk=segment_id).delete()

api/tests/unit/projects/test_tasks.py → api/tests/unit/projects/test_unit_projects_tasks.py

@@ -1,9 +1,17 @@
 import pytest
+from pytest_django.fixtures import SettingsWrapper
 from pytest_mock import MockerFixture
 
 from environments.dynamodb.types import IdentityOverridesV2Changeset
+from environments.models import Environment
+from features.models import Feature
 from projects.models import IdentityOverridesV2MigrationStatus, Project
-from projects.tasks import migrate_project_environments_to_v2
+from projects.tasks import (
+    handle_cascade_delete,
+    migrate_project_environments_to_v2,
+)
+from segments.models import Segment
+from task_processor.task_run_method import TaskRunMethod
 
 
 @pytest.fixture
@@ -88,3 +96,22 @@ def test_migrate_project_environments_to_v2__expected_status_on_error(
     assert project_v2_migration_in_progress.identity_overrides_v2_migration_status == (
         IdentityOverridesV2MigrationStatus.IN_PROGRESS
     )
+
+
+def test_handle_cascade_delete(
+    project: Project,
+    environment: Environment,
+    feature: Feature,
+    segment: Segment,
+    settings: SettingsWrapper,
+) -> None:
+    # Given
+    settings.TASK_RUN_METHOD = TaskRunMethod.SYNCHRONOUSLY
+
+    # When
+    handle_cascade_delete(project_id=project.id)
+
+    # Then
+    assert not Environment.objects.filter(id=environment.id).exists()
+    assert not Feature.objects.filter(id=feature.id).exists()
+    assert not Segment.objects.filter(id=segment.id).exists()

api/tests/unit/projects/test_unit_projects_views.py

@@ -5,7 +5,9 @@
 import pytest
 from django.urls import reverse
 from django.utils import timezone
+from pytest_django.fixtures import SettingsWrapper
 from pytest_lazyfixture import lazy_fixture
+from pytest_mock import MockerFixture
 from rest_framework import status
 from rest_framework.test import APIClient
 
@@ -30,6 +32,7 @@
     VIEW_PROJECT,
 )
 from segments.models import Segment
+from task_processor.task_run_method import TaskRunMethod
 from users.models import FFAdminUser, UserPermissionGroup
 
 now = timezone.now()
@@ -783,3 +786,25 @@ def test_get_project_data_by_id(
     assert response_json["total_features"] == num_features
     assert response_json["total_segments"] == num_segments
     assert response_json["show_edge_identity_overrides_for_feature"] is False
+
+
+def test_delete_project_delete_handles_cascade_delete(
+    admin_client: APIClient,
+    project: Project,
+    mocker: MockerFixture,
+    settings: SettingsWrapper,
+) -> None:
+    # Given
+    settings.TASK_RUN_METHOD = TaskRunMethod.SYNCHRONOUSLY
+
+    url = reverse("api-v1:projects:project-detail", args=[project.id])
+    mocked_handle_cascade_delete = mocker.patch("projects.models.handle_cascade_delete")
+
+    # When
+    response = admin_client.delete(url)
+
+    # Then
+    assert response.status_code == status.HTTP_204_NO_CONTENT
+    mocked_handle_cascade_delete.delay.assert_called_once_with(
+        kwargs={"project_id": project.id}
+    )

api/tests/unit/task_processor/test_unit_task_processor_models.py

@@ -1,15 +1,29 @@
-from datetime import timedelta
+from datetime import datetime, time, timedelta
 from decimal import Decimal
 
 import pytest
 from django.utils import timezone
+from freezegun import freeze_time
+from pytest_lazyfixture import lazy_fixture
 
 from task_processor.decorators import register_task_handler
 from task_processor.models import RecurringTask, Task
 
-now = timezone.now()
-one_hour_ago = now - timedelta(hours=1)
-one_hour_from_now = now + timedelta(hours=1)
+
+@pytest.fixture
+def current_datetime() -> datetime:
+    with freeze_time("2024-01-31T09:45:16.758115"):
+        yield timezone.now()
+
+
+@pytest.fixture
+def one_hour_ago(current_datetime: datetime) -> time:
+    return (current_datetime - timedelta(hours=1)).time()
+
+
+@pytest.fixture
+def one_hour_from_now(current_datetime: datetime) -> time:
+    return (current_datetime + timedelta(hours=1)).time()
 
 
 @register_task_handler()
@@ -50,7 +64,7 @@ def test_serialize_data_handles_decimal_objects(input, expected_output):
 
 @pytest.mark.parametrize(
     "first_run_time, expected",
-    ((one_hour_ago.time(), True), (one_hour_from_now.time(), False)),
+    ((lazy_fixture("one_hour_ago"), True), (lazy_fixture("one_hour_from_now"), False)),
 )
 def test_recurring_task_run_should_execute_first_run_at(first_run_time, expected):
     assert (