Skip to content

Commit

Permalink
fix: Long DELETE project call
Browse files Browse the repository at this point in the history
  • Loading branch information
khvn26 committed Jan 31, 2024
1 parent 81a5e59 commit 8afd597
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 7 deletions.
20 changes: 20 additions & 0 deletions api/environments/migrations/0034_alter_environment_project.py
Original file line number Diff line number Diff line change
@@ -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'),
),
]
15 changes: 11 additions & 4 deletions api/environments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,14 @@ class Environment(
" Environment. New default Feature States will be created for the new"
" selected projects Features for this Environment."
),
on_delete=models.CASCADE,
# Deleting large environments take significant amount of queries due to
# audit log signals and lifecycle events for related objects.
# For now, decouple environment deletion from project deletion
# to be able to perform it asynchronously.
#
# We choose `DO_NOTHING` here as we're only taking project soft deletes
# into account
on_delete=models.DO_NOTHING,
)

api_key = models.CharField(
Expand Down Expand Up @@ -461,16 +468,16 @@ def natural_key(self):
def is_valid(self) -> bool:
return self.active and (not self.expires_at or self.expires_at > timezone.now())

@hook(AFTER_SAVE, when="_should_update_dynamo", is_now=True)
@hook(AFTER_SAVE, when="should_update_dynamo", is_now=True)
def send_to_dynamo(self):
environment_api_key_wrapper.write_api_key(self)

@hook(AFTER_DELETE, when="_should_update_dynamo", is_now=True)
@hook(AFTER_DELETE, when="should_update_dynamo", is_now=True)
def delete_from_dynamo(self):
environment_api_key_wrapper.delete_api_key(self.key)

@property
def _should_update_dynamo(self) -> bool:
def should_update_dynamo(self) -> bool:
return (
self.environment.project.enable_dynamo_db
and environment_api_key_wrapper.is_enabled
Expand Down
5 changes: 5 additions & 0 deletions api/environments/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
10 changes: 7 additions & 3 deletions api/projects/views.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.conf import settings
from django.utils.decorators import method_decorator
from drf_yasg import openapi
Expand All @@ -16,6 +13,7 @@
from environments.dynamodb.migrator import IdentityMigrator
from environments.identities.models import Identity
from environments.serializers import EnvironmentSerializerLight
from environments.tasks import delete_environment
from permissions.permissions_calculator import get_project_permission_data
from permissions.serializers import (
PermissionModelSerializer,
Expand Down Expand Up @@ -178,6 +176,12 @@ def migrate_to_edge(self, request: Request, pk: int = None):
identity_migrator.trigger_migration()
return Response(status=status.HTTP_202_ACCEPTED)

def perform_destroy(self, instance: Project) -> None:
instance.delete()

for environment_id in instance.environments.values_list("id", flat=True):
delete_environment.delay(kwargs={"environment_id": environment_id})


class BaseProjectPermissionsViewSet(viewsets.ModelViewSet):
model_class = None
Expand Down
24 changes: 24 additions & 0 deletions api/tests/unit/projects/test_unit_projects_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
from django.urls import reverse
from django.utils import timezone
from pytest_lazyfixture import lazy_fixture
from pytest_mock import MockerFixture
from rest_framework import status
from rest_framework.test import APIClient

from environments.dynamodb.types import ProjectIdentityMigrationStatus
from environments.identities.models import Identity
from environments.models import Environment
from features.models import Feature, FeatureSegment
from organisations.models import Organisation, OrganisationRole
from organisations.permissions.models import (
Expand Down Expand Up @@ -783,3 +785,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__calls_expected(
admin_client: APIClient,
project: Project,
environment: Environment,
mocker: MockerFixture,
) -> None:
# Given
url = reverse("api-v1:projects:project-detail", args=[project.id])

delete_environment_mock = mocker.patch("projects.views.delete_environment")

# When
admin_client.delete(url)

# Then
project.refresh_from_db()
assert project.deleted
delete_environment_mock.delay.assert_called_once_with(
kwargs={"environment_id": environment.id}
)

0 comments on commit 8afd597

Please sign in to comment.