From e4d886b41c2087a3583d29dd912522a57d5b5327 Mon Sep 17 00:00:00 2001 From: Jorge Date: Thu, 14 Nov 2024 14:08:30 -0500 Subject: [PATCH 1/2] Implement PACS query API endpoint --- chris_backend/core/api.py | 19 ++- .../pacsfiles/migrations/0003_pacsquery.py | 33 +++++ chris_backend/pacsfiles/models.py | 37 +++++ chris_backend/pacsfiles/permissions.py | 31 ++++ chris_backend/pacsfiles/serializers.py | 58 +++++++- chris_backend/pacsfiles/services.py | 7 +- chris_backend/pacsfiles/views.py | 139 +++++++++++++++++- 7 files changed, 317 insertions(+), 7 deletions(-) create mode 100644 chris_backend/pacsfiles/migrations/0003_pacsquery.py diff --git a/chris_backend/core/api.py b/chris_backend/core/api.py index 4cfea440..5332f66e 100755 --- a/chris_backend/core/api.py +++ b/chris_backend/core/api.py @@ -31,6 +31,7 @@ path('v1/users//groups/', user_views.UserGroupList.as_view(), name='user-group-list'), + path('v1/groups/', user_views.GroupList.as_view(), name='group-list'), @@ -373,6 +374,22 @@ pacsfile_views.PACSDetail.as_view(), name='pacs-detail'), + path('v1/pacs//queries/', + pacsfile_views.PACSQueryList.as_view(), + name='pacsquery-list'), + + path('v1/pacs/queries/', + pacsfile_views.AllPACSQueryList.as_view(), + name='allpacsquery-list'), + + path('v1/pacs/queries/search/', + pacsfile_views.AllPACSQueryListQuerySearch.as_view(), + name='allpacsquery-list-query-search'), + + path('v1/pacs/queries//', + pacsfile_views.PACSQueryDetail.as_view(), + name='pacsquery-detail'), + path('v1/pacs//series/', pacsfile_views.PACSSpecificSeriesList.as_view(), name='pacs-specific-series-list'), @@ -523,4 +540,4 @@ # Login and logout views for Djangos' browsable API urlpatterns += [ path('v1/auth/', include('rest_framework.urls', namespace='rest_framework')), -] \ No newline at end of file +] diff --git a/chris_backend/pacsfiles/migrations/0003_pacsquery.py b/chris_backend/pacsfiles/migrations/0003_pacsquery.py new file mode 100644 index 00000000..bff35849 --- /dev/null +++ b/chris_backend/pacsfiles/migrations/0003_pacsquery.py @@ -0,0 +1,33 @@ +# Generated by Django 4.2.5 on 2024-11-05 21:13 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('pacsfiles', '0002_pacs_active'), + ] + + operations = [ + migrations.CreateModel( + name='PACSQuery', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('creation_date', models.DateTimeField(auto_now_add=True)), + ('title', models.CharField(db_index=True, max_length=300)), + ('query', models.JSONField()), + ('description', models.CharField(blank=True, max_length=700)), + ('result', models.TextField(blank=True)), + ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ('pacs', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='query_list', to='pacsfiles.pacs')), + ], + options={ + 'ordering': ('pacs', 'owner', '-creation_date'), + 'unique_together': {('pacs', 'owner', 'title')}, + }, + ), + ] diff --git a/chris_backend/pacsfiles/models.py b/chris_backend/pacsfiles/models.py index fd776f1d..7eb91184 100755 --- a/chris_backend/pacsfiles/models.py +++ b/chris_backend/pacsfiles/models.py @@ -43,6 +43,43 @@ class Meta: fields = ['id', 'identifier', 'active'] +class PACSQuery(models.Model): + creation_date = models.DateTimeField(auto_now_add=True) + title = models.CharField(max_length=300, db_index=True) + query = models.JSONField() + description = models.CharField(max_length=700, blank=True) + result = models.TextField(blank=True) + pacs = models.ForeignKey(PACS, on_delete=models.CASCADE, related_name='query_list') + owner = models.ForeignKey('auth.User', on_delete=models.CASCADE) + + class Meta: + ordering = ('pacs', 'owner', '-creation_date',) + unique_together = ('pacs', 'owner', 'title',) + + def __str__(self): + return self.query + + +class PACSQueryFilter(FilterSet): + min_creation_date = django_filters.IsoDateTimeFilter(field_name='creation_date', + lookup_expr='gte') + max_creation_date = django_filters.IsoDateTimeFilter(field_name='creation_date', + lookup_expr='lte') + title_exact = django_filters.CharFilter(field_name='title', lookup_expr='exact') + title = django_filters.CharFilter(field_name='title', lookup_expr='icontains') + description = django_filters.CharFilter(field_name='description', + lookup_expr='icontains') + pacs_identifier = django_filters.CharFilter(field_name='pacs__identifier', + lookup_expr='exact') + owner_username = django_filters.CharFilter(field_name='owner__username', + lookup_expr='exact') + + class Meta: + model = PACSQuery + fields = ['id', 'min_creation_date', 'max_creation_date', 'title_exact', + 'title', 'description', 'pacs_identifier', 'owner_username'] + + class PACSSeries(models.Model): creation_date = models.DateTimeField(auto_now_add=True) PatientID = models.CharField(max_length=100, db_index=True) diff --git a/chris_backend/pacsfiles/permissions.py b/chris_backend/pacsfiles/permissions.py index a948bddb..3886796e 100755 --- a/chris_backend/pacsfiles/permissions.py +++ b/chris_backend/pacsfiles/permissions.py @@ -16,3 +16,34 @@ def has_permission(self, request, view): return (request.method in permissions.SAFE_METHODS and user.groups.filter( name='pacs_users').exists()) + + +class IsChrisOrIsPACSUserOrReadOnly(permissions.BasePermission): + """ + Custom permission to only allow superuser 'chris' and other users in the pacs_users + group to create objects. Read only is allowed to all other users. + """ + + def has_permission(self, request, view): + user = request.user + + if request.method in permissions.SAFE_METHODS: + return True + + return user.username == 'chris' or user.groups.filter(name='pacs_users').exists() + + +class IsChrisOrOwnerOrIsPACSUserReadOnly(permissions.BasePermission): + """ + Custom permission to only allow superuser 'chris' to create it. + Read only is allowed to other users in the pacs_users group. + """ + + def has_object_permission(self, request, view, obj): + user = request.user + + if user.username == 'chris' or user == obj.owner: + return True + + return (request.method in permissions.SAFE_METHODS and user.groups.filter( + name='pacs_users').exists()) diff --git a/chris_backend/pacsfiles/serializers.py b/chris_backend/pacsfiles/serializers.py index a228fd3b..f104d967 100755 --- a/chris_backend/pacsfiles/serializers.py +++ b/chris_backend/pacsfiles/serializers.py @@ -3,6 +3,7 @@ import os import time +from django.db.utils import IntegrityError from django.contrib.auth.models import Group from django.conf import settings from rest_framework import serializers @@ -10,8 +11,10 @@ from core.models import ChrisFolder from core.storage import connect_storage from core.serializers import ChrisFileSerializer +from core.utils import json_zip2str -from .models import PACS, PACSSeries, PACSFile +from .models import PACS, PACSQuery, PACSSeries, PACSFile +from .services import PfdcmClient logger = logging.getLogger(__name__) @@ -30,6 +33,59 @@ class Meta: 'pacs_series_list') +class PACSQuerySerializer(serializers.HyperlinkedModelSerializer): + query = serializers.JSONField(binary=True) + result = serializers.ReadOnlyField() + pacs_identifier = serializers.ReadOnlyField(source='pacs.identifier') + owner_username = serializers.ReadOnlyField(source='owner.username') + + class Meta: + model = PACSQuery + fields = ('url', 'id', 'creation_date', 'title', 'query', 'description', + 'result', 'pacs_identifier', 'owner_username') + + def create(self, validated_data): + """ + Overriden to rise a serializer error when attempting to create a PACSQuery + object that results in a DB conflict. Then a query is made to the PFDCM service. + """ + title = validated_data['title'] + query = validated_data['query'] + pacs_name = validated_data['pacs'].identifier + + try: + pacs_query = super(PACSQuerySerializer, self).create(validated_data) + except IntegrityError: + error_msg = (f'You have already registered a PACS query with title={title} ' + f'for pacs {pacs_name}') + raise serializers.ValidationError([error_msg]) + + pfdcm_cl = PfdcmClient() + result = pfdcm_cl.query(pacs_name, query) + + if result: + pacs_query.result = json_zip2str(result) + pacs_query.save() + return pacs_query + + def update(self, instance, validated_data): + """ + Overriden to rise a serializer error when attempting to update a PACSQuery + object that results in a DB conflict. + """ + pacs = instance.pacs + title = validated_data.get('title') + + if title is None: + title = instance.title + try: + return super(PACSQuerySerializer, self).update(instance, validated_data) + except IntegrityError: + error_msg = (f'You have already registered a PACS query with title={title} ' + f'for pacs {pacs.identifier}') + raise serializers.ValidationError([error_msg]) + + class PACSSeriesSerializer(serializers.HyperlinkedModelSerializer): path = serializers.CharField(max_length=1024, write_only=True) folder_path = serializers.ReadOnlyField(source='folder.path') diff --git a/chris_backend/pacsfiles/services.py b/chris_backend/pacsfiles/services.py index da5d8a5f..61fa125f 100755 --- a/chris_backend/pacsfiles/services.py +++ b/chris_backend/pacsfiles/services.py @@ -63,7 +63,12 @@ def query(self, pacs_name, query, timeout=30): raise time.sleep(0.4) else: - return resp.json() + result = resp.json() + if result.get('status'): + pypx = result.get('pypx') + if pypx and 'data' in pypx: + return pypx['data'] + return [] def retrieve(self, pacs_name, query, timeout=30): """ diff --git a/chris_backend/pacsfiles/views.py b/chris_backend/pacsfiles/views.py index e10f38f9..e1819556 100755 --- a/chris_backend/pacsfiles/views.py +++ b/chris_backend/pacsfiles/views.py @@ -10,11 +10,13 @@ from core.renderers import BinaryFileRenderer from core.models import ChrisFolder from core.views import TokenAuthSupportQueryString -from .models import (PACS, PACSFilter, PACSSeries, PACSSeriesFilter, PACSFile, - PACSFileFilter) -from .serializers import PACSSerializer, PACSSeriesSerializer, PACSFileSerializer +from .models import (PACS, PACSFilter, PACSQuery, PACSQueryFilter, PACSSeries, + PACSSeriesFilter, PACSFile, PACSFileFilter) +from .serializers import (PACSSerializer, PACSQuerySerializer, PACSSeriesSerializer, + PACSFileSerializer) from .services import PfdcmClient -from .permissions import IsChrisOrIsPACSUserReadOnly +from .permissions import (IsChrisOrIsPACSUserReadOnly, IsChrisOrIsPACSUserOrReadOnly, + IsChrisOrOwnerOrIsPACSUserReadOnly) class PACSList(generics.ListCreateAPIView): @@ -125,6 +127,135 @@ def get_pacs_series_queryset(self): return self.filter_queryset(pacs.series_list.all()) +class PACSQueryList(generics.ListCreateAPIView): + """ + A view for the collection of PACS-specific queries. + """ + http_method_names = ['get', 'post'] + queryset = PACS.objects.all() + serializer_class = PACSQuerySerializer + permission_classes = (permissions.IsAuthenticated, IsChrisOrIsPACSUserOrReadOnly,) + + def list(self, request, *args, **kwargs): + """ + Overriden to return the list of queries for the queried pacs. A document-level + link relation and a collection+json template are also added to the response. + """ + queryset = self.get_pacs_queries_queryset() + response = services.get_list_response(self, queryset) + pacs = self.get_object() + + # append document-level link relations + links = {'pacs': reverse('pacs-detail', request=request, + kwargs={"pk": pacs.id})} + response = services.append_collection_links(response, links) + + # append write template + template_data = {'title': '', 'query': '', 'description': ''} + return services.append_collection_template(response, template_data) + + def get_pacs_queries_queryset(self): + """ + Custom method to get the actual PACS queries' queryset. The returned queryset + is limited to only the queries owned by the user when the user is no longer in + the pacs_users group. + """ + user = self.request.user + pacs = self.get_object() + + if user.username == 'chris' or user.groups.filter(name='pacs_users').exists(): + return pacs.query_list.all() + return pacs.query_list.filter(owner=self.request.user) + + def perform_create(self, serializer): + """ + Overriden to associate the owner and the pacs with the PACS query before first + saving to the DB. + """ + pacs = self.get_object() + serializer.save(owner=self.request.user, pacs=pacs) + + +class AllPACSQueryList(generics.ListAPIView): + """ + A view for the collection of all pacs queries. + """ + http_method_names = ['get'] + serializer_class = PACSQuerySerializer + permission_classes = (permissions.IsAuthenticated,) + + def list(self, request, *args, **kwargs): + """ + Overriden to add a query list and document-level link relation to the response. + """ + response = super(AllPACSQueryList, self).list(request, *args, **kwargs) + # append query list + query_list = [reverse('allpacsquery-list-query-search', request=request)] + response = services.append_collection_querylist(response, query_list) + # append document-level link relations + links = {'pacs': reverse('pacs-list', request=request)} + return services.append_collection_links(response, links) + + def get_queryset(self): + """ + Overriden to limit the returned queryset to only the queries owned by the user + when the user is no longer in the pacs_users group. + """ + user = self.request.user + + if user.username == 'chris' or user.groups.filter(name='pacs_users').exists(): + return PACSQuery.objects.all() + return PACSQuery.objects.filter(owner=user) + + +class AllPACSQueryListQuerySearch(generics.ListAPIView): + """ + A view for the collection of workflows resulting from a query search. + """ + http_method_names = ['get'] + serializer_class = PACSQuerySerializer + permission_classes = (permissions.IsAuthenticated,) + filterset_class = PACSQueryFilter + + def get_queryset(self): + """ + Overriden to limit the returned queryset to only the queries owned by the user + when the user is no longer in the pacs_users group. + """ + user = self.request.user + + if user.username == 'chris' or user.groups.filter(name='pacs_users').exists(): + return PACSQuery.objects.all() + return PACSQuery.objects.filter(owner=user) + + +class PACSQueryDetail(generics.RetrieveUpdateDestroyAPIView): + """ + A PACS query view. + """ + http_method_names = ['get', 'put', 'delete'] + queryset = PACSQuery.objects.all() + serializer_class = PACSQuerySerializer + permission_classes = (permissions.IsAuthenticated, IsChrisOrOwnerOrIsPACSUserReadOnly) + + def retrieve(self, request, *args, **kwargs): + """ + Overriden to append a collection+json template to the response. + """ + response = super(PACSQueryDetail, self).retrieve(request, *args, **kwargs) + template_data = {'title': '', 'description': ''} + return services.append_collection_template(response, template_data) + + def update(self, request, *args, **kwargs): + """ + Overriden to remove descriptors that are not allowed to be updated before + serializer validation. + """ + data = self.request.data + data.pop('query', None) + return super(PACSQueryDetail, self).update(request, *args, **kwargs) + + class PACSSeriesList(generics.ListCreateAPIView): """ A view for the collection of all PACS Series. From 121454cf1f2f982576c606e1fc695de57a57313f Mon Sep 17 00:00:00 2001 From: Jorge Luis Bernal Rusiel Date: Tue, 19 Nov 2024 18:19:24 -0500 Subject: [PATCH 2/2] Add a comprehensive set of tests for the PACS API endpoints --- chris_backend/feeds/views.py | 1 + chris_backend/pacsfiles/serializers.py | 18 +- .../pacsfiles/tests/test_serializers.py | 97 ++++- chris_backend/pacsfiles/tests/test_views.py | 363 +++++++++++++++++- chris_backend/pacsfiles/views.py | 2 +- 5 files changed, 464 insertions(+), 17 deletions(-) diff --git a/chris_backend/feeds/views.py b/chris_backend/feeds/views.py index ddadbf03..8063d951 100755 --- a/chris_backend/feeds/views.py +++ b/chris_backend/feeds/views.py @@ -329,6 +329,7 @@ def list(self, request, *args, **kwargs): request=request), 'userfiles': reverse('userfile-list', request=request), 'pacs': reverse('pacs-list', request=request), + 'pacsqueries': reverse('allpacsquery-list', request=request), 'pacsfiles': reverse('pacsfile-list', request=request), 'pacsseries': reverse('pacsseries-list', request=request), 'filebrowser': reverse('chrisfolder-list', request=request)} diff --git a/chris_backend/pacsfiles/serializers.py b/chris_backend/pacsfiles/serializers.py index f104d967..d002583f 100755 --- a/chris_backend/pacsfiles/serializers.py +++ b/chris_backend/pacsfiles/serializers.py @@ -24,17 +24,17 @@ class PACSSerializer(serializers.HyperlinkedModelSerializer): folder_path = serializers.ReadOnlyField(source='folder.path') folder = serializers.HyperlinkedRelatedField(view_name='chrisfolder-detail', read_only=True) - pacs_series_list = serializers.HyperlinkedIdentityField( - view_name='pacs-specific-series-list') + query_list = serializers.HyperlinkedIdentityField(view_name='pacsquery-list') + series_list = serializers.HyperlinkedIdentityField(view_name='pacs-specific-series-list') class Meta: model = PACS fields = ('url', 'id', 'identifier', 'active', 'folder_path', 'folder', - 'pacs_series_list') + 'query_list', 'series_list') class PACSQuerySerializer(serializers.HyperlinkedModelSerializer): - query = serializers.JSONField(binary=True) + query = serializers.JSONField(binary=True, required=False) result = serializers.ReadOnlyField() pacs_identifier = serializers.ReadOnlyField(source='pacs.identifier') owner_username = serializers.ReadOnlyField(source='owner.username') @@ -85,6 +85,16 @@ def update(self, instance, validated_data): f'for pacs {pacs.identifier}') raise serializers.ValidationError([error_msg]) + def validate(self, data): + """ + Overriden to validate that the query field is in data when creating a new query. + """ + if not self.instance: # on create + if 'query' not in data: + raise serializers.ValidationError( + {'query': ["This field is required."]}) + return data + class PACSSeriesSerializer(serializers.HyperlinkedModelSerializer): path = serializers.CharField(max_length=1024, write_only=True) diff --git a/chris_backend/pacsfiles/tests/test_serializers.py b/chris_backend/pacsfiles/tests/test_serializers.py index 80985d5e..25ef3555 100755 --- a/chris_backend/pacsfiles/tests/test_serializers.py +++ b/chris_backend/pacsfiles/tests/test_serializers.py @@ -7,27 +7,114 @@ from unittest import mock from rest_framework import serializers -from pacsfiles.serializers import PACSSeriesSerializer +from core.models import ChrisFolder +from core.utils import json_zip2str +from pacsfiles.models import PACS, PACSQuery +from pacsfiles.serializers import PACSQuerySerializer, PACSSeriesSerializer CHRIS_SUPERUSER_PASSWORD = settings.CHRIS_SUPERUSER_PASSWORD -class PACSSeriesSerializerTests(TestCase): - +class SerializerTests(TestCase): def setUp(self): # avoid cluttered console output (for instance logging all the http requests) logging.disable(logging.WARNING) - # create superuser chris (owner of root folders) + # superuser chris (owner of root folders) self.chris_username = 'chris' - self.chris_password = CHRIS_SUPERUSER_PASSWORD + chris_user = User.objects.get(username=self.chris_username) + + # create normal user + self.username = 'foo' + self.password = 'bar' + User.objects.create_user(username=self.username, password=self.password) + + # create a PACS + self.pacs_name = 'myPACS' + folder_path = f'SERVICES/PACS/{self.pacs_name}' + (pacs_folder, tf) = ChrisFolder.objects.get_or_create(path=folder_path, + owner=chris_user) + PACS.objects.get_or_create(folder=pacs_folder, identifier=self.pacs_name) def tearDown(self): # re-enable logging logging.disable(logging.NOTSET) +class PACSQuerySerializerTests(SerializerTests): + + def test_create_success(self): + """ + Test whether overriden 'create' method successfully creates a new PACS query. + """ + user = User.objects.get(username=self.username) + pacs = PACS.objects.get(identifier=self.pacs_name) + query = {'SeriesInstanceUID': '2.3.15.2.1057'} + data = {'title': 'query1', 'query': query, 'owner': user, 'pacs': pacs} + + with mock.patch('pacsfiles.serializers.PfdcmClient.query') as pfdcm_query_mock: + result = {'mock': 'mock'} + pfdcm_query_mock.return_value = result + pacs_query_serializer = PACSQuerySerializer(data=data) + pacs_query = pacs_query_serializer.create(data) + pfdcm_query_mock.assert_called_with(self.pacs_name, query) + self.assertEqual(pacs_query.result, json_zip2str(result)) + + + def test_create_failure_pacs_user_title_combination_already_exists(self): + """ + Test whether overriden 'create' method raises a ValidationError when a user has + already registered a PACS query with the same title and pacs. + """ + user = User.objects.get(username=self.username) + pacs = PACS.objects.get(identifier=self.pacs_name) + query = {'SeriesInstanceUID': '1.3.12.2.1107'} + + PACSQuery.objects.get_or_create(title='query2', query=query, owner=user, pacs=pacs) + + data = {'title': 'query2', 'query': query, 'owner': user, 'pacs': pacs} + pacs_query_serializer = PACSQuerySerializer(data=data) + with self.assertRaises(serializers.ValidationError): + pacs_query_serializer.create(data) + + def test_update_success(self): + """ + Test whether overriden 'update' method successfully updates an existing PACS query. + """ + user = User.objects.get(username=self.username) + pacs = PACS.objects.get(identifier=self.pacs_name) + query = {'SeriesInstanceUID': '2.3.15.2.1057'} + + pacs_query, _ = PACSQuery.objects.get_or_create(title='query2', query=query, + owner=user, pacs=pacs) + + data = {'title': 'query4'} + pacs_query_serializer = PACSQuerySerializer(pacs_query, data) + pacs_query = pacs_query_serializer.update(pacs_query, data) + self.assertEqual(pacs_query.title, 'query4') + + def test_update_failure_pacs_user_title_combination_already_exists(self): + """ + Test whether overriden 'update' method raises a ValidationError when a user has + already registered a PACS query with the same title and pacs. + """ + user = User.objects.get(username=self.username) + pacs = PACS.objects.get(identifier=self.pacs_name) + query = {'SeriesInstanceUID': '1.3.12.2.1107'} + + pacs_query, _ = PACSQuery.objects.get_or_create(title='query2', query=query, + owner=user, pacs=pacs) + PACSQuery.objects.get_or_create(title='query3', query=query, owner=user, pacs=pacs) + + data = {'title': 'query3'} + pacs_query_serializer = PACSQuerySerializer(pacs_query, data) + with self.assertRaises(serializers.ValidationError): + pacs_query_serializer.update(pacs_query, data) + + +class PACSSeriesSerializerTests(SerializerTests): + def test_validate_ndicom_failure_not_positive(self): """ Test whether overriden validate_ndicom method validates submitted ndicom must diff --git a/chris_backend/pacsfiles/tests/test_views.py b/chris_backend/pacsfiles/tests/test_views.py index f64c2766..e59baf3a 100755 --- a/chris_backend/pacsfiles/tests/test_views.py +++ b/chris_backend/pacsfiles/tests/test_views.py @@ -13,7 +13,7 @@ from core.models import ChrisFolder from core.storage import connect_storage -from pacsfiles.models import PACS, PACSSeries, PACSFile +from pacsfiles.models import PACS, PACSQuery, PACSSeries, PACSFile from pacsfiles import views @@ -36,11 +36,15 @@ def setUp(self): self.content_type = 'application/vnd.collection+json' self.username = 'test' self.password = 'testpass' + self.other_username = 'boo' + self.other_password = 'far' pacs_grp, _ = Group.objects.get_or_create(name='pacs_users') user = User.objects.create_user(username=self.username, password=self.password) user.groups.set([pacs_grp]) + User.objects.create_user(username=self.other_username, password=self.other_password) + # create a PACS file in the DB "already registered" to the server) self.storage_manager = connect_storage(settings) # upload file to storage @@ -103,6 +107,26 @@ def test_pacs_list_failure_unauthenticated(self): self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) +class PACSListQuerySearchViewTests(PACSViewTests): + """ + Test the pacs-list-query-search view. + """ + + def setUp(self): + super(PACSListQuerySearchViewTests, self).setUp() + + self.read_url = reverse("pacs-list-query-search") + '?name=' + self.pacs_name + + def test_pacs_list_query_search_success(self): + self.client.login(username=self.username, password=self.password) + response = self.client.get(self.read_url) + self.assertContains(response, self.pacs_name) + + def test_pacs_list_query_search_failure_unauthenticated(self): + response = self.client.get(self.read_url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + class PACSDetailViewTests(PACSViewTests): """ Test the pacs-detail view. @@ -144,6 +168,217 @@ def test_pacs_list_failure_unauthenticated(self): self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) +class PACSQueryListViewTests(PACSViewTests): + """ + Test the pacsquery-list view. + """ + + def setUp(self): + super(PACSQueryListViewTests, self).setUp() + + pacs = PACS.objects.get(identifier=self.pacs_name) + user = User.objects.get(username=self.username) + + self.create_read_url = reverse("pacsquery-list", kwargs={"pk": pacs.id}) + + query = {'SeriesInstanceUID': '2.3.15.2.1057'} + pacs_query, _ = PACSQuery.objects.get_or_create(title='query1', query=query, + owner=user, pacs=pacs) + + self.post = json.dumps( + {"template": {"data": [{"name": "title", "value": 'test1'}, + {"name": "query", "value": json.dumps(query)}]}}) + + def test_pacs_query_list_success(self): + self.client.login(username=self.username, password=self.password) + response = self.client.get(self.create_read_url) + self.assertContains(response, 'query1') + + def test_pacs_query_list_success_readonly(self): + pacs = PACS.objects.get(identifier=self.pacs_name) + user = User.objects.get(username=self.other_username) + + query = {'SeriesInstanceUID': '2.3.15.2.1158'} + pacs_query, _ = PACSQuery.objects.get_or_create(title='query2', query=query, + owner=user, pacs=pacs) + self.client.login(username=self.other_username, password=self.other_password) # not a member of pacs_users + response = self.client.get(self.create_read_url) + self.assertContains(response, 'query2') # can see its own queries + self.assertNotContains(response, 'query1') # cannot see other users' queries + + def test_pacs_query_list_failure_unauthenticated(self): + response = self.client.get(self.create_read_url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_pacs_query_create_success(self): + self.client.login(username=self.username, password=self.password) + response = self.client.post(self.create_read_url, data=self.post, + content_type=self.content_type) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + def test_pacs_query_create_failure_unauthenticated(self): + response = self.client.post(self.create_read_url, data=self.post, + content_type=self.content_type) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_pacs_query_create_failure_forbidden(self): + self.client.login(username=self.other_username, password=self.other_password) + response = self.client.post(self.create_read_url, data=self.post, + content_type=self.content_type) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + +class AllPACSQueryListViewTests(PACSQueryListViewTests): + """ + Test the allpacsquery-list view. + """ + + def setUp(self): + super(AllPACSQueryListViewTests, self).setUp() + + self.read_url = reverse("allpacsquery-list") + + def test_all_pacs_query_list_success(self): + self.client.login(username=self.username, password=self.password) + response = self.client.get(self.read_url) + self.assertContains(response, 'query1') + + def test_all_pacs_query_list_success_readonly(self): + pacs = PACS.objects.get(identifier=self.pacs_name) + user = User.objects.get(username=self.other_username) + + query = {'SeriesInstanceUID': '2.3.15.2.1158'} + pacs_query, _ = PACSQuery.objects.get_or_create(title='query2', query=query, + owner=user, pacs=pacs) + self.client.login(username=self.other_username, password=self.other_password) # not a member of pacs_users + response = self.client.get(self.read_url) + self.assertContains(response, 'query2') # can see its own queries + self.assertNotContains(response, 'query1') # cannot see other users' queries + + def test_all_pacs_query_list_failure_unauthenticated(self): + response = self.client.get(self.read_url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + +class AllPACSQueryListQuerySearchViewTests(PACSQueryListViewTests): + """ + Test the allpacsquery-list-query-search view. + """ + + def setUp(self): + super(AllPACSQueryListQuerySearchViewTests, self).setUp() + + self.read_url = reverse("allpacsquery-list-query-search") + '?name=query1' + + def test_all_pacs_query_list_query_search_success(self): + self.client.login(username=self.username, password=self.password) + response = self.client.get(self.read_url) + self.assertContains(response, 'query1') + + def test_all_pacs_query_list_query_search_success_readonly(self): + self.client.login(username=self.other_username, password=self.other_password) + response = self.client.get(self.read_url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['results'], []) + + def test_all_pacs_query_list_query_search_failure_unauthenticated(self): + response = self.client.get(self.read_url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + +class PACSQueryDetailViewTests(PACSViewTests): + """ + Test the pacsquery-detail view. + """ + + def setUp(self): + super(PACSQueryDetailViewTests, self).setUp() + + self.other_pacs_user_username = 'loo' + self.other_pacs_user_password = 'lar' + + pacs_grp, _ = Group.objects.get_or_create(name='pacs_users') + pacs_user = User.objects.create_user(username=self.other_pacs_user_username, + password=self.other_pacs_user_password) + pacs_user.groups.set([pacs_grp]) + + pacs = PACS.objects.get(identifier=self.pacs_name) + user = User.objects.get(username=self.username) + query = {'SeriesInstanceUID': '2.3.15.2.1057'} + + pacs_query, _ = PACSQuery.objects.get_or_create(title='query1', query=query, + owner=user, pacs=pacs) + self.read_update_delete_url = reverse("pacsquery-detail", + kwargs={"pk":pacs_query.id}) + + self.put = json.dumps({ + "template": {"data": [{"name": "title", "value": "Test query"}]}}) + + def test_pacs_query_detail_success(self): + self.client.login(username=self.username, password=self.password) + response = self.client.get(self.read_update_delete_url) + self.assertContains(response, 'query1') + + def test_pacs_query_detail_success_other_pacs_user(self): + self.client.login(username=self.other_pacs_user_username, + password=self.other_pacs_user_password) + response = self.client.get(self.read_update_delete_url) + self.assertContains(response, 'query1') + + def test_pacs_query_detail_failure_unauthenticated(self): + response = self.client.get(self.read_update_delete_url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_pacs_query_detail_failure_forbidden(self): + self.client.login(username=self.other_username, password=self.other_password) + response = self.client.get(self.read_update_delete_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_pacs_query_update_success(self): + self.client.login(username=self.username, password=self.password) + response = self.client.put(self.read_update_delete_url, data=self.put, + content_type=self.content_type) + self.assertContains(response, "Test query") + + def test_pacs_query_update_failure_unauthenticated(self): + response = self.client.put(self.read_update_delete_url, data=self.put, + content_type=self.content_type) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_pacs_query_update_failure_access_denied_non_pacs_user(self): + self.client.login(username=self.other_username, password=self.other_password) + response = self.client.put(self.read_update_delete_url, data=self.put, + content_type=self.content_type) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_pacs_query_update_failure_access_denied_other_pacs_user(self): + self.client.login(username=self.other_pacs_user_username, + password=self.other_pacs_user_password) + response = self.client.put(self.read_update_delete_url, data=self.put, + content_type=self.content_type) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_pacs_query_delete_success(self): + self.client.login(username=self.username, password=self.password) + response = self.client.delete(self.read_update_delete_url) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + + def test_pacs_query_delete_failure_unauthenticated(self): + response = self.client.delete(self.read_update_delete_url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_pacs_query_delete_failure_access_denied_non_pacs_user(self): + self.client.login(username=self.other_username, password=self.other_password) + response = self.client.delete(self.read_update_delete_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_pacs_query_delete_failure_access_denied_other_pacs_user(self): + self.client.login(username=self.other_pacs_user_username, + password=self.other_pacs_user_password) + response = self.client.delete(self.read_update_delete_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + class PACSSeriesListViewTests(PACSViewTests): """ Test the pacsseries-list view. @@ -151,8 +386,37 @@ class PACSSeriesListViewTests(PACSViewTests): def setUp(self): super(PACSSeriesListViewTests, self).setUp() + + # create a PACS file in the DB "already registered" to the server) + self.storage_manager = connect_storage(settings) + + # upload file to storage + self.new_path = 'SERVICES/PACS/MyPACS/123456-good/brain_good_study/SAG_T1_MPRAGE' + with io.StringIO("test file") as file1: + self.storage_manager.upload_obj(self.new_path + '/file1.dcm', file1.read(), + content_type='text/plain') + self.post = json.dumps( + {"template": {"data": [{"name": "path", "value": self.new_path + '/file1.dcm'}, + {"name": "ndicom", "value" : 1}, + {"name": "PatientID", "value": "123456"}, + {"name": "PatientName", "value": "crazy"}, + {"name": "PatientBirthDate", "value": "2020-07-15"}, + {"name": "PatientSex", "value": "O"}, + {"name": "StudyDate", "value": "2020-07-15"}, + {"name": "StudyInstanceUID", "value": "1.1.3432.54.6545674765.765478"}, + {"name": "StudyDescription", "value": "brain_good_study"}, + {"name": "SeriesInstanceUID", "value": "2.4.3432.54.845674765.763357"}, + {"name": "SeriesDescription", "value": "SAG T1 MPRAGE"}, + {"name": "pacs_name", "value": self.pacs_name} + ]}}) + self.create_read_url = reverse("pacsseries-list") + def tearDown(self): + # delete file from storage + self.storage_manager.delete_obj(self.new_path + '/file1.dcm') + super(PACSSeriesListViewTests, self).tearDown() + def test_pacs_series_list_success(self): self.client.login(username=self.username, password=self.password) response = self.client.get(self.create_read_url) @@ -162,10 +426,57 @@ def test_pacs_series_list_failure_unauthenticated(self): response = self.client.get(self.create_read_url) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + def test_pacs_series_list_failure_access_denied(self): + self.client.login(username=self.other_username, password=self.other_password) + response = self.client.get(self.create_read_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_pacs_series_create_success(self): + self.client.login(username=self.chris_username, password=self.chris_password) + response = self.client.post(self.create_read_url, data=self.post, + content_type=self.content_type) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + def test_pacs_series_create_failure_unauthenticated(self): + response = self.client.post(self.create_read_url, data=self.post, + content_type=self.content_type) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_pacs_series_create_failure_forbidden(self): + self.client.login(username=self.username, password=self.password) + response = self.client.post(self.create_read_url, data=self.post, + content_type=self.content_type) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + +class PACSSeriesListQuerySearchViewTests(PACSQueryListViewTests): + """ + Test the pacsseries-list-query-search view. + """ + + def setUp(self): + super(PACSSeriesListQuerySearchViewTests, self).setUp() + + self.read_url = reverse("pacsseries-list-query-search") + '?PatientID=123456' + + def test_pacs_series_list_query_search_success(self): + self.client.login(username=self.username, password=self.password) + response = self.client.get(self.read_url) + self.assertContains(response, 'crazy') + + def test_pacs_series_list_query_search_failure_unauthenticated(self): + response = self.client.get(self.read_url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_pacs_series_list_query_search_failure_access_denied(self): + self.client.login(username=self.other_username, password=self.other_password) + response = self.client.get(self.read_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + class PACSSeriesDetailViewTests(PACSViewTests): """ - Test the pacsfile-detail view. + Test the pacsseries-detail view. """ def setUp(self): @@ -183,6 +494,11 @@ def test_pacs_series_detail_failure_unauthenticated(self): response = self.client.get(self.read_url) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + def test_pacs_series_detail_failure_access_denied(self): + self.client.login(username=self.other_username, password=self.other_password) + response = self.client.get(self.read_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + class PACSFileListViewTests(PACSViewTests): """ @@ -193,8 +509,29 @@ def setUp(self): super(PACSFileListViewTests, self).setUp() self.read_url = reverse("pacsfile-list") - def tearDown(self): - super(PACSFileListViewTests, self).tearDown() + def test_pacsfile_list_success(self): + self.client.login(username=self.username, password=self.password) + response = self.client.get(self.read_url) + self.assertContains(response, self.path) + + def test_pacsfile_list_failure_unauthenticated(self): + response = self.client.get(self.read_url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_pacsfile_list_failure_access_denied(self): + self.client.login(username=self.other_username, password=self.other_password) + response = self.client.get(self.read_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + +class PACSFileListQuerySearchViewTests(PACSViewTests): + """ + Test the pacsfile-list-query-search view. + """ + + def setUp(self): + super(PACSFileListQuerySearchViewTests, self).setUp() + self.read_url = reverse("pacsseries-list-query-search") + '?path=' + self.path def test_pacsfile_list_success(self): self.client.login(username=self.username, password=self.password) @@ -205,6 +542,11 @@ def test_pacsfile_list_failure_unauthenticated(self): response = self.client.get(self.read_url) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + def test_pacsfile_list_failure_access_denied(self): + self.client.login(username=self.other_username, password=self.other_password) + response = self.client.get(self.read_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + class PACSFileDetailViewTests(PACSViewTests): """ @@ -226,6 +568,11 @@ def test_pacsfile_detail_failure_unauthenticated(self): response = self.client.get(self.read_url) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + def test_pacsfile_detail_failure_access_denied(self): + self.client.login(username=self.other_username, password=self.other_password) + response = self.client.get(self.read_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + class PACSFileResourceViewTests(PACSViewTests): """ @@ -238,9 +585,6 @@ def setUp(self): self.download_url = reverse("pacsfile-resource", kwargs={"pk": pacs_file.id}) + 'file1.dcm' - def tearDown(self): - super(PACSFileResourceViewTests, self).tearDown() - def test_pacsfileresource_get(self): pacs_file = PACSFile.objects.get(fname=self.path + '/file1.dcm') fileresource_view_inst = mock.Mock() @@ -261,3 +605,8 @@ def test_integration_pacsfileresource_download_success(self): def test_fileresource_download_failure_unauthenticated(self): response = self.client.get(self.download_url) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_fileresource_download_failure_access_denied(self): + self.client.login(username=self.other_username, password=self.other_password) + response = self.client.get(self.download_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) diff --git a/chris_backend/pacsfiles/views.py b/chris_backend/pacsfiles/views.py index e1819556..1b4950b6 100755 --- a/chris_backend/pacsfiles/views.py +++ b/chris_backend/pacsfiles/views.py @@ -19,7 +19,7 @@ IsChrisOrOwnerOrIsPACSUserReadOnly) -class PACSList(generics.ListCreateAPIView): +class PACSList(generics.ListAPIView): """ A view for the collection of registered PACS. """