Skip to content

Commit 785b562

Browse files
authored
Merge pull request #690 from jbernal0019/master
Implement async delete for PACS series, feeds, plugin instances and folders
2 parents f4b8160 + 183e55d commit 785b562

26 files changed

Lines changed: 730 additions & 57 deletions

chris_backend/core/celery.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@
4040
{'queue': 'periodic'},
4141
'plugininstances.tasks.delete_plugin_instances_jobs_from_remote':
4242
{'queue': 'periodic'},
43+
'plugininstances.tasks.delete_plugin_instance': {'queue': 'main2'},
44+
'feeds.tasks.delete_feed': {'queue': 'main2'},
45+
'filebrowser.tasks.delete_folder': {'queue': 'main2'},
46+
'pacsfiles.tasks.delete_pacs_series': {'queue': 'main2'},
4347
'pacsfiles.tasks.send_pacs_query': {'queue': 'main2'},
4448
'pacsfiles.tasks.register_pacs_series': {'queue': 'main2'}
4549
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# Generated by Django 5.2.9 on 2026-02-12 00:16
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
('core', '0002_chrisfile_public_chrisfolder_public_and_more'),
10+
]
11+
12+
operations = [
13+
migrations.AddField(
14+
model_name='chrisfolder',
15+
name='deletion_error',
16+
field=models.TextField(blank=True),
17+
),
18+
migrations.AddField(
19+
model_name='chrisfolder',
20+
name='deletion_requested_at',
21+
field=models.DateTimeField(blank=True, null=True),
22+
),
23+
migrations.AddField(
24+
model_name='chrisfolder',
25+
name='deletion_status',
26+
field=models.CharField(choices=[('inactive', 'Inactive'), ('pending', 'Pending'), ('failed', 'Failed')], default='inactive', max_length=10),
27+
),
28+
]

chris_backend/core/models.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from django.db import models
88
from django.db.models.functions import Length
99
from django.db.models.signals import post_delete
10+
from django.utils import timezone
1011
from django.dispatch import receiver
1112
from django.conf import settings
1213
from django.contrib.auth.models import User, Group
@@ -35,6 +36,42 @@ def validate_permission(permission):
3536
return permission
3637

3738

39+
class AsyncDeletableModel(models.Model):
40+
class DeletionStatus(models.TextChoices):
41+
INACTIVE = 'inactive'
42+
PENDING = 'pending'
43+
FAILED = 'failed'
44+
45+
deletion_status = models.CharField(max_length=10, choices=DeletionStatus.choices,
46+
default=DeletionStatus.INACTIVE)
47+
deletion_requested_at = models.DateTimeField(null=True, blank=True)
48+
deletion_error = models.TextField(blank=True)
49+
50+
class Meta:
51+
abstract = True
52+
53+
def is_pending_deletion(self) -> bool:
54+
return self.deletion_status == self.DeletionStatus.PENDING
55+
56+
def mark_deletion_pending(self) -> bool:
57+
"""
58+
Returns True if deletion was scheduled by this call.
59+
Returns False if it was already pending.
60+
"""
61+
if self.is_pending_deletion():
62+
return False
63+
64+
self.deletion_status = self.DeletionStatus.PENDING
65+
self.deletion_requested_at = timezone.now()
66+
self.save(update_fields=['deletion_status', 'deletion_requested_at'])
67+
return True
68+
69+
def mark_deletion_failed(self, error: Exception | str):
70+
self.deletion_status = self.DeletionStatus.FAILED
71+
self.deletion_error = str(error)
72+
self.save(update_fields=['deletion_status', 'deletion_error'])
73+
74+
3875
class ChrisInstance(models.Model):
3976
"""
4077
Model class that defines a singleton representing a ChRIS instance.
@@ -71,7 +108,7 @@ def load(cls):
71108
return obj
72109

73110

74-
class ChrisFolder(models.Model):
111+
class ChrisFolder(AsyncDeletableModel):
75112
creation_date = models.DateTimeField(auto_now_add=True)
76113
path = models.CharField(max_length=1024, unique=True) # folder's path
77114
public = models.BooleanField(blank=True, default=False, db_index=True)
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# Generated by Django 5.2.9 on 2026-02-12 00:16
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
('feeds', '0002_remove_feed_owner_feeduserpermission_and_more'),
10+
]
11+
12+
operations = [
13+
migrations.AddField(
14+
model_name='feed',
15+
name='deletion_error',
16+
field=models.TextField(blank=True),
17+
),
18+
migrations.AddField(
19+
model_name='feed',
20+
name='deletion_requested_at',
21+
field=models.DateTimeField(blank=True, null=True),
22+
),
23+
migrations.AddField(
24+
model_name='feed',
25+
name='deletion_status',
26+
field=models.CharField(choices=[('inactive', 'Inactive'), ('pending', 'Pending'), ('failed', 'Failed')], default='inactive', max_length=10),
27+
),
28+
]

chris_backend/feeds/models.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@
88
import django_filters
99
from django_filters.rest_framework import FilterSet
1010

11-
from core.models import (ChrisFolder, ChrisFile, ChrisLinkFile, FolderGroupPermission,
12-
FolderUserPermission, FileGroupPermission, FileUserPermission,
13-
LinkFileGroupPermission, LinkFileUserPermission)
11+
from core.models import (AsyncDeletableModel, ChrisFolder, ChrisFile, ChrisLinkFile,
12+
FolderGroupPermission, FolderUserPermission, FileGroupPermission,
13+
FileUserPermission, LinkFileGroupPermission,
14+
LinkFileUserPermission)
1415
from userfiles.models import UserFile
1516

1617

17-
class Feed(models.Model):
18+
class Feed(AsyncDeletableModel):
1819
creation_date = models.DateTimeField(auto_now_add=True)
1920
modification_date = models.DateTimeField(auto_now_add=True)
2021
name = models.CharField(max_length=200, blank=True, db_index=True)
@@ -197,7 +198,8 @@ class FeedFilter(FilterSet):
197198
class Meta:
198199
model = Feed
199200
fields = ['id', 'name', 'name_exact', 'name_startswith', 'min_id', 'max_id',
200-
'min_creation_date', 'max_creation_date', 'files_fname_icontains']
201+
'min_creation_date', 'max_creation_date', 'files_fname_icontains',
202+
'deletion_status']
201203

202204
def filter_by_fname_icontains(self, queryset, name, value):
203205
"""

chris_backend/feeds/serializers.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ class Meta:
118118
fields = ('url', 'id', 'creation_date', 'modification_date', 'name', 'public',
119119
'owner_username', 'folder_path', 'created_jobs', 'waiting_jobs',
120120
'scheduled_jobs', 'started_jobs', 'registering_jobs',
121-
'finished_jobs', 'errored_jobs', 'cancelled_jobs', 'folder', 'note',
121+
'finished_jobs', 'errored_jobs', 'cancelled_jobs', 'deletion_status',
122+
'deletion_requested_at', 'deletion_error', 'folder', 'note',
122123
'group_permissions', 'user_permissions', 'tags', 'taggings',
123124
'comments', 'plugin_instances', 'owner')
124125

chris_backend/feeds/tasks.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
2+
import logging
3+
4+
from celery import shared_task
5+
from .models import Feed
6+
7+
8+
logger = logging.getLogger(__name__)
9+
10+
11+
@shared_task(bind=True, autoretry_for=(Exception,), retry_kwargs={"max_retries": 3})
12+
def delete_feed(self, feed_id):
13+
try:
14+
feed = Feed.objects.get(id=feed_id)
15+
16+
if not feed.is_pending_deletion():
17+
return # idempotent safety
18+
19+
feed.delete()
20+
except Feed.DoesNotExist:
21+
pass
22+
except Exception as e:
23+
Feed.objects.filter(id=feed_id).update( # atomic update
24+
deletion_status=Feed.DeletionStatus.FAILED,
25+
deletion_error=str(e)
26+
)
27+
raise

chris_backend/feeds/tests/test_views.py

Lines changed: 95 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,24 @@
11

22
import logging
33
import json
4+
import time
5+
from unittest import mock
46

5-
from django.test import TestCase
7+
from django.test import TestCase, TransactionTestCase, tag
68
from django.urls import reverse
79
from django.contrib.auth.models import User, Group
810
from django.conf import settings
911
from rest_framework import status
1012

13+
from celery.contrib.testing.worker import start_worker
14+
from core.celery import app as celery_app
15+
from core.celery import task_routes
16+
1117
from plugins.models import PluginMeta, Plugin, ComputeResource
1218
from plugininstances.models import PluginInstance
1319
from feeds.models import (Note, Tag, Tagging, Feed, FeedGroupPermission,
1420
FeedUserPermission, Comment)
21+
from feeds import views
1522

1623

1724
COMPUTE_RESOURCE_URL = settings.COMPUTE_RESOURCE_URL
@@ -79,7 +86,75 @@ def setUp(self):
7986
def tearDown(self):
8087
# re-enable logging
8188
logging.disable(logging.NOTSET)
82-
89+
90+
91+
class TasksViewTests(TransactionTestCase):
92+
93+
@classmethod
94+
def setUpClass(cls):
95+
logging.disable(logging.WARNING)
96+
super().setUpClass()
97+
# route tasks to this worker by using the default 'celery' queue
98+
# that is exclusively used for the automated tests
99+
celery_app.conf.update(task_routes=None)
100+
cls.celery_worker = start_worker(celery_app,
101+
concurrency=1,
102+
perform_ping_check=False)
103+
cls.celery_worker.__enter__()
104+
105+
@classmethod
106+
def tearDownClass(cls):
107+
super().tearDownClass()
108+
cls.celery_worker.__exit__(None, None, None)
109+
# reset routes to the original queues
110+
celery_app.conf.update(task_routes=task_routes)
111+
logging.disable(logging.NOTSET)
112+
113+
def setUp(self):
114+
# create superuser chris (owner of root folders)
115+
self.chris_username = 'chris'
116+
self.chris_password = CHRIS_SUPERUSER_PASSWORD
117+
118+
self.content_type = 'application/vnd.collection+json'
119+
120+
self.username = 'foo'
121+
self.password = 'foopass'
122+
self.other_username = 'booo'
123+
self.other_password = 'booopass'
124+
125+
self.plugin_name = "pacspull"
126+
self.plugin_type = "fs"
127+
self.plugin_parameters = {'mrn': {'type': 'string', 'optional': False},
128+
'img_type': {'type': 'string', 'optional': True}}
129+
self.feedname = "Feed1"
130+
131+
(self.compute_resource, tf) = ComputeResource.objects.get_or_create(
132+
name="host", compute_url=COMPUTE_RESOURCE_URL)
133+
134+
# create users
135+
other_user = User.objects.create_user(username=self.other_username,
136+
password=self.other_password)
137+
user = User.objects.create_user(username=self.username,
138+
password=self.password)
139+
140+
# assign predefined group
141+
all_grp = Group.objects.get(name='all_users')
142+
143+
other_user.groups.set([all_grp])
144+
user.groups.set([all_grp])
145+
146+
# create plugin
147+
(pl_meta, tf) = PluginMeta.objects.get_or_create(name='pacspull', type='fs')
148+
(plugin, tf) = Plugin.objects.get_or_create(meta=pl_meta, version='0.1')
149+
plugin.compute_resources.set([self.compute_resource])
150+
plugin.save()
151+
152+
# create a feed by creating a "fs" plugin instance
153+
pl_inst = PluginInstance.objects.create(plugin=plugin, owner=user, title='test',
154+
compute_resource=
155+
plugin.compute_resources.all()[0])
156+
pl_inst.feed.name = self.feedname
157+
pl_inst.feed.save()
83158

84159
class NoteDetailViewTests(ViewTests):
85160
"""
@@ -224,7 +299,7 @@ def test_feed_list_query_search_from_other_users_not_listed(self):
224299
self.assertNotContains(response, "Feed2")
225300

226301

227-
class FeedDetailViewTests(ViewTests):
302+
class FeedDetailViewTests(TasksViewTests):
228303
"""
229304
Test the feed-detail view.
230305
"""
@@ -274,8 +349,24 @@ def test_feed_update_failure_access_denied(self):
274349

275350
def test_feed_delete_success(self):
276351
self.client.login(username=self.username, password=self.password)
352+
353+
with mock.patch.object(views.delete_feed, 'delay',
354+
return_value=None) as delay_mock:
355+
response = self.client.delete(self.read_update_delete_url)
356+
self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED)
357+
358+
# check that the delete_feed task was called with appropriate args
359+
delay_mock.assert_called_with(response.data['id'])
360+
361+
@tag('integration')
362+
def test_integration_feed_delete_success(self):
363+
self.client.login(username=self.username, password=self.password)
277364
response = self.client.delete(self.read_update_delete_url)
278-
self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
365+
self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED)
366+
367+
for _ in range(10):
368+
time.sleep(3)
369+
if Feed.objects.count() == 0: break
279370
self.assertEqual(Feed.objects.count(), 0)
280371

281372
def test_feed_delete_failure_unauthenticated(self):

chris_backend/feeds/views.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11

22
from django.db.models import Q
33
from django.shortcuts import get_object_or_404
4-
from rest_framework import generics, permissions
4+
from rest_framework import generics, permissions, status
5+
from rest_framework.response import Response
56
from rest_framework.reverse import reverse
67
from drf_spectacular.utils import extend_schema, extend_schema_view
78

@@ -14,6 +15,7 @@
1415
from .serializers import (FeedSerializer, FeedGroupPermissionSerializer,
1516
FeedUserPermissionSerializer, NoteSerializer,
1617
TagSerializer, TaggingSerializer, CommentSerializer)
18+
from .tasks import delete_feed
1719
from .permissions import (
1820
IsChrisOrFeedOwnerOrHasFeedPermissionOrPublicFeedReadOnly, IsOwnerOrChrisOrReadOnly,
1921
IsOwnerOrChrisOrHasPermissionOrPublicReadOnly, IsOwnerOrChrisOrHasPermissionReadOnly,
@@ -389,6 +391,19 @@ def retrieve(self, request, *args, **kwargs):
389391
template_data = {"name": "", "public": ""}
390392
return services.append_collection_template(response, template_data)
391393

394+
def destroy(self, request, *args, **kwargs):
395+
"""
396+
Overriden to asyncronously delete the feed and its folder/files from
397+
storage.
398+
"""
399+
instance = self.get_object()
400+
401+
if instance.mark_deletion_pending():
402+
delete_feed.delay(instance.id) # async task
403+
404+
serializer = self.get_serializer(instance)
405+
return Response(serializer.data, status=status.HTTP_202_ACCEPTED)
406+
392407

393408
class PublicFeedList(generics.ListAPIView):
394409
"""

chris_backend/filebrowser/serializers.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class FileBrowserFolderSerializer(serializers.HyperlinkedModelSerializer):
3333
class Meta:
3434
model = ChrisFolder
3535
fields = ('url', 'id', 'creation_date', 'path', 'public', 'owner_username',
36+
'deletion_status', 'deletion_requested_at', 'deletion_error',
3637
'parent', 'children', 'files', 'link_files', 'group_permissions',
3738
'user_permissions', 'owner')
3839

0 commit comments

Comments
 (0)