diff --git a/arkindex/documents/api/elements.py b/arkindex/documents/api/elements.py
index 4e95292bff86b183a92ac84a0cbecf66930d57ac..b8d30c77ce88305ca3f24b5b13b57fe106643f7c 100644
--- a/arkindex/documents/api/elements.py
+++ b/arkindex/documents/api/elements.py
@@ -10,7 +10,7 @@ from django.db.models.functions import Cast
from django.shortcuts import get_object_or_404
from django.utils.functional import cached_property
from psycopg2.extras import execute_values
-from rest_framework import serializers, status
+from rest_framework import permissions, serializers, status
from rest_framework.exceptions import NotFound, ValidationError
from rest_framework.generics import (
CreateAPIView,
@@ -21,6 +21,7 @@ from rest_framework.generics import (
RetrieveUpdateDestroyAPIView,
UpdateAPIView,
)
+from rest_framework.mixins import DestroyModelMixin
from rest_framework.response import Response
from arkindex.dataimport.models import WorkerVersion
@@ -56,7 +57,7 @@ from arkindex.project.openapi import AutoSchema
from arkindex.project.pagination import PageNumberPagination
from arkindex.project.permissions import IsAuthenticated, IsVerified, IsVerifiedOrReadOnly
from arkindex.project.tools import BulkMap
-from arkindex.project.triggers import corpus_delete
+from arkindex.project.triggers import corpus_delete, element_trash
from arkindex_common.enums import TranscriptionType
classifications_queryset = Classification.objects.select_related('ml_class', 'source').order_by('-confidence')
@@ -124,6 +125,7 @@ class ElementsListMixin(object):
"""
serializer_class = ElementListSerializer
queryset = Element.objects.all()
+ permission_classes = (IsVerifiedOrReadOnly, )
openapi_overrides = {
'security': [],
'tags': ['elements'],
@@ -135,7 +137,7 @@ class ElementsListMixin(object):
'required': False,
'schema': {
'type': 'string',
- }
+ },
},
{
'name': 'name',
@@ -153,7 +155,8 @@ class ElementsListMixin(object):
'schema': {'anyOf': [
{'type': 'string', 'format': 'uuid'},
{'type': 'boolean'},
- ]}
+ ]},
+ 'x-methods': ['get'],
},
{
'name': 'folder',
@@ -185,7 +188,8 @@ class ElementsListMixin(object):
'schema': {
'type': 'boolean',
'default': False,
- }
+ },
+ 'x-methods': ['get'],
},
{
'name': 'with_has_children',
@@ -198,7 +202,8 @@ class ElementsListMixin(object):
'schema': {
'type': 'boolean',
'default': False
- }
+ },
+ 'x-methods': ['get'],
},
{
'name': 'If-Modified-Since',
@@ -214,20 +219,66 @@ class ElementsListMixin(object):
'schema': {
# Cannot use format: date-time here as HTTP headers do not use ISO 8601
'type': 'string'
- }
+ },
+ 'x-methods': ['get'],
+ },
+ {
+ 'name': 'delete_children',
+ 'required': False,
+ 'in': 'query',
+ 'description': 'Delete all child elements of those elements recursively.',
+ 'schema': {
+ 'type': 'boolean',
+ 'default': True
+ },
+ 'x-methods': ['delete'],
}
]
}
+ def initial(self, *args, **kwargs):
+ """
+ Adds a `self.clean_params` attribute that holds any query params or headers,
+ filtered by their allowed HTTP method.
+ Raise HTTP 400 early if any filter is not allowed in this method.
+ Does not perform validation on the filter values.
+ """
+ super().initial(*args, **kwargs)
+
+ self.clean_params = {}
+ errors = {}
+ for param in self.openapi_overrides['parameters']:
+ if param['in'] == 'header':
+ origin_dict = self.request.headers
+ elif param['in'] == 'query':
+ origin_dict = self.request.query_params
+ else:
+ raise NotImplementedError
+
+ if param['name'] not in origin_dict:
+ continue
+
+ if param.get('x-methods') and self.request.method.lower() not in param['x-methods']:
+ errors[param['name']] = [f'This parameter is not allowed in {self.request.method} requests.']
+ else:
+ self.clean_params[param['name']] = origin_dict[param['name']]
+
+ if errors:
+ raise ValidationError(errors)
+
@cached_property
def selected_corpus(self):
corpus_id = self.kwargs.get('corpus')
if corpus_id is None:
return
- # When a corpus is specified, check independently that it's readable
- # by the current user. This prevents comparing the corpus of every element
- return self.get_corpus(corpus_id)
+ # When a corpus is specified, check independently that it's readable/writable
+ # by the current user. This prevents comparing the corpus of every element.
+ # Require write rights for 'unsafe' methods (POST/PUT/PATCH/DELETE)
+ return self.get_corpus(
+ corpus_id,
+ right=Right.Read if self.request.method in permissions.SAFE_METHODS else Right.Write,
+ )
def get_filters(self):
filters = {}
@@ -237,16 +288,16 @@ class ElementsListMixin(object):
filters['corpus__in'] = Corpus.objects.readable(self.request.user)
if 'name' in self.request.query_params:
- filters['name__icontains'] = self.request.query_params['name']
+ filters['name__icontains'] = self.clean_params['name']
if 'type' in self.request.query_params:
- filters['type__slug'] = self.request.query_params['type']
+ filters['type__slug'] = self.clean_params['type']
- only_folder = self.request.query_params.get('folder')
+ only_folder = self.clean_params.get('folder')
if only_folder is not None:
filters['type__folder'] = only_folder.lower() not in ('false', '0')
- if 'worker_version' in self.request.query_params:
+ if 'worker_version' in self.clean_params:
try:
worker_version_id = uuid.UUID(self.request.query_params['worker_version'])
except (TypeError, ValueError):
@@ -267,7 +318,7 @@ class ElementsListMixin(object):
- elements with any best classes
- elements with a specific best class
"""
- class_filter = self.request.query_params.get('best_class')
+ class_filter = self.clean_params.get('best_class')
if class_filter is None:
return
@@ -292,7 +343,7 @@ class ElementsListMixin(object):
def get_prefetch(self):
prefetch = {'corpus', 'zone__image__server', 'type'}
- with_best_classes = self.request.query_params.get('with_best_classes')
+ with_best_classes = self.clean_params.get('with_best_classes')
if with_best_classes and with_best_classes.lower() not in ('false', '0'):
prefetch.add(best_classifications_prefetch)
@@ -313,7 +364,7 @@ class ElementsListMixin(object):
# Use queryset.distinct() whenever best_class is defined
queryset = queryset.filter(class_filters).distinct()
- with_has_children = self.request.query_params.get('with_has_children')
+ with_has_children = self.clean_params.get('with_has_children')
if with_has_children and with_has_children.lower() not in ('false', '0'):
queryset = BulkMap(_fetch_has_children, queryset)
@@ -328,7 +379,7 @@ class ElementsListMixin(object):
queryset = queryset.iterable
assert isinstance(queryset, QuerySet), 'A Django QuerySet is required to check for modified elements'
- modified_since_string = self.request.headers.get('If-Modified-Since')
+ modified_since_string = self.clean_params.get('If-Modified-Since')
if not modified_since_string:
return False
@@ -358,6 +409,16 @@ class ElementsListMixin(object):
serializer = self.get_serializer(queryset, many=True)
return Response(serializer.data)
+ def delete(self, request, *args, **kwargs):
+ queryset = self.filter_queryset(self.get_queryset())
+ delete_children = self.clean_params.get('delete_children', '').lower() not in ('false', '0')
+
+ if not queryset.exists():
+ raise NotFound
+
+ element_trash(queryset, delete_children=delete_children)
+ return Response(status=status.HTTP_204_NO_CONTENT)
+
class DeprecatedElementsList(DeprecatedMixin, RetrieveAPIView):
"""
@@ -376,7 +437,7 @@ class DeprecatedElementsList(DeprecatedMixin, RetrieveAPIView):
)
-class CorpusElements(ElementsListMixin, CorpusACLMixin, ListAPIView):
+class CorpusElements(ElementsListMixin, CorpusACLMixin, DestroyModelMixin, ListAPIView):
"""
List elements in a corpus and filter by type, name, ML class
"""
@@ -396,7 +457,7 @@ class CorpusElements(ElementsListMixin, CorpusACLMixin, ListAPIView):
@property
def is_top_level(self):
- return self.request.query_params.get('top_level') not in (None, 'false', '0')
+ return self.clean_params.get('top_level') not in (None, 'false', '0')
def get_queryset(self):
# Should not be possible due to the URL
@@ -421,7 +482,7 @@ class CorpusElements(ElementsListMixin, CorpusACLMixin, ListAPIView):
return super().get_order_by()
-class ElementParents(ElementsListMixin, ListAPIView):
+class ElementParents(ElementsListMixin, DestroyModelMixin, ListAPIView):
"""
List all parents of an element
"""
@@ -445,14 +506,22 @@ class ElementParents(ElementsListMixin, ListAPIView):
@property
def is_recursive(self):
- recursive_param = self.request.query_params.get('recursive')
+ recursive_param = self.clean_params.get('recursive')
return recursive_param is not None and recursive_param.lower() not in ('false', '0')
def get_queryset(self):
+ if self.request.method in permissions.SAFE_METHODS:
+ corpora = Corpus.objects.readable(self.request.user)
+ else:
+ corpora = Corpus.objects.writable(self.request.user)
+
+ if not Element.objects.filter(id=self.kwargs['pk'], corpus__in=corpora).exists():
+ raise NotFound
+
return Element.objects.get_ascending(self.kwargs['pk'], recursive=self.is_recursive)
-class ElementChildren(ElementsListMixin, ListAPIView):
+class ElementChildren(ElementsListMixin, DestroyModelMixin, ListAPIView):
"""
List all children of an element
"""
@@ -476,7 +545,7 @@ class ElementChildren(ElementsListMixin, ListAPIView):
def get_filters(self):
filters = super().get_filters()
- recursive_param = self.request.query_params.get('recursive')
+ recursive_param = self.clean_params.get('recursive')
if recursive_param is None or recursive_param.lower() in ('false', '0'):
# Only list direct children
@@ -492,6 +561,14 @@ class ElementChildren(ElementsListMixin, ListAPIView):
return ('paths__ordering', )
def get_queryset(self):
+ if self.request.method in permissions.SAFE_METHODS:
+ corpora = Corpus.objects.readable(self.request.user)
+ else:
+ corpora = Corpus.objects.writable(self.request.user)
+
+ if not Element.objects.filter(id=self.kwargs['pk'], corpus__in=corpora).exists():
+ raise NotFound
+
return Element.objects.get_descending(self.kwargs['pk'])
diff --git a/arkindex/documents/managers.py b/arkindex/documents/managers.py
index 7aa35ba64caedb5bcc4cb4e4559809f153e2888c..be83286798c03a2d2cd11bead087cf1d7d08d19c 100644
--- a/arkindex/documents/managers.py
+++ b/arkindex/documents/managers.py
@@ -1,13 +1,24 @@
import uuid
from itertools import chain, groupby
-from django.db import connections, models
+import django
+from django.db import DJANGO_VERSION_PICKLE_KEY, connections, models
from arkindex.project.fields import Unnest
class ElementQuerySet(models.QuerySet):
+ def __getstate__(self):
+ """
+ Django's QuerySets can be pickled, but when they are, they first call self._fetch_all()
+ to fetch every row into the result cache and store it along with the pickled data,
+ because one could expect to 'save' a queryset that way.
+ We only pickle querysets for RQ tasks that will call .trash() later;
+ this will never use the result cache as it does not load rows into memory, so we ignore that step.
+ """
+ return {**self.__dict__, DJANGO_VERSION_PICKLE_KEY: django.__version__}
+
def trash(self, delete_children=True):
"""Performant deletion of any element queryset"""
from arkindex.documents.models import Element, MetaData, Transcription, TranscriptionEntity, Classification, Selection
diff --git a/arkindex/documents/tasks.py b/arkindex/documents/tasks.py
index b178a81698f7f2f646478d8811e2fdafb3980297..8d0c98502a5061fbec1285f8c2c62e4145bec089 100644
--- a/arkindex/documents/tasks.py
+++ b/arkindex/documents/tasks.py
@@ -8,6 +8,7 @@ from django_rq import job
from arkindex.dataimport.models import DataImport, DataImportElement, WorkerRun
from arkindex.documents.indexer import Indexer
+from arkindex.documents.managers import ElementQuerySet
from arkindex.documents.models import (
Classification,
Corpus,
@@ -193,3 +194,8 @@ def corpus_delete(corpus_id: str) -> None:
logger.info(f'Deleted {deleted_count} {queryset.model.__name__}')
logger.info(f'Deleted corpus {corpus_id}')
+
+
+@job('high')
+def element_trash(queryset: ElementQuerySet, delete_children: bool) -> None:
+ queryset.trash(delete_children=delete_children)
diff --git a/arkindex/documents/tests/test_children_elements.py b/arkindex/documents/tests/test_children_elements.py
index e78f7a002885642f939cebd711c2b15ca3b6045e..df19a1b9f6dd5af637bb95bfb0fc1f721b484e50 100644
--- a/arkindex/documents/tests/test_children_elements.py
+++ b/arkindex/documents/tests/test_children_elements.py
@@ -104,7 +104,7 @@ class TestChildrenElements(FixtureAPITestCase):
def test_element_children_worker_version(self):
self.corpus.elements.filter(name__contains='page 1r').update(worker_version=self.worker_version)
- with self.assertNumQueries(8):
+ with self.assertNumQueries(9):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
data={'worker_version': str(self.worker_version.id)}
@@ -118,7 +118,7 @@ class TestChildrenElements(FixtureAPITestCase):
)
def test_element_children_worker_version_validation(self):
- with self.assertNumQueries(0):
+ with self.assertNumQueries(1):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
data={'worker_version': 'blah'},
@@ -126,7 +126,7 @@ class TestChildrenElements(FixtureAPITestCase):
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.json(), {'worker_version': ['Invalid UUID']})
- with self.assertNumQueries(1):
+ with self.assertNumQueries(2):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
data={'worker_version': uuid.uuid4()},
@@ -163,7 +163,7 @@ class TestChildrenElements(FixtureAPITestCase):
)
element.add_parent(self.vol)
- with self.assertNumQueries(9):
+ with self.assertNumQueries(10):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
data={'with_has_children': True},
@@ -185,7 +185,7 @@ class TestChildrenElements(FixtureAPITestCase):
)
def test_children_modified_since_bad_format(self):
- with self.assertNumQueries(0):
+ with self.assertNumQueries(1):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
HTTP_IF_MODIFIED_SINCE='AAAAAAAAAAAAAAAAA',
@@ -194,7 +194,7 @@ class TestChildrenElements(FixtureAPITestCase):
self.assertDictEqual(response.json(), {'If-Modified-Since': ['Bad date format']})
def test_children_modified_since_not_modified(self):
- with self.assertNumQueries(1):
+ with self.assertNumQueries(2):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
HTTP_IF_MODIFIED_SINCE='Thu, 02 Apr 2099 13:37:42 GMT',
@@ -203,7 +203,7 @@ class TestChildrenElements(FixtureAPITestCase):
def test_children_modified_since(self):
self.corpus.elements.filter(name='Volume 1, page 1r').update(updated='2099-04-02T13:37:43Z')
- with self.assertNumQueries(8):
+ with self.assertNumQueries(9):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
HTTP_IF_MODIFIED_SINCE='Thu, 02 Apr 2099 13:37:42 GMT',
diff --git a/arkindex/documents/tests/test_classes.py b/arkindex/documents/tests/test_classes.py
index 6136b229d95de1e481cfca84914113ea2492a671..4984fef2ee2260147d3820905f99a995f7807382 100644
--- a/arkindex/documents/tests/test_classes.py
+++ b/arkindex/documents/tests/test_classes.py
@@ -265,7 +265,7 @@ class TestClasses(FixtureAPITestCase):
def test_element_parents_best_classes(self):
self.populate_classified_elements()
- with self.assertNumQueries(6):
+ with self.assertNumQueries(7):
response = self.client.get(
reverse('api:elements-parents', kwargs={'pk': str(self.common_children.id)}),
data={'type': self.classified.slug, 'with_best_classes': 1}
@@ -281,7 +281,7 @@ class TestClasses(FixtureAPITestCase):
def test_element_children_best_classes(self):
self.populate_classified_elements()
- with self.assertNumQueries(6):
+ with self.assertNumQueries(7):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': str(self.parent.id)}),
data={'type': self.classified.slug, 'with_best_classes': 'yes'}
@@ -302,7 +302,7 @@ class TestClasses(FixtureAPITestCase):
self.populate_classified_elements()
child = Element.objects.filter(type=self.classified.id).first()
child.classifications.all().update(state=ClassificationState.Rejected)
- with self.assertNumQueries(6):
+ with self.assertNumQueries(7):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': str(self.parent.id)}),
data={'type': self.classified.slug, 'with_best_classes': 'yes'}
@@ -405,7 +405,7 @@ class TestClasses(FixtureAPITestCase):
self.populate_classified_elements()
parent = Element.objects.get_ascending(self.common_children.id).last()
parent.classifications.all().filter(confidence=.7).update(state=ClassificationState.Validated)
- with self.assertNumQueries(5):
+ with self.assertNumQueries(6):
response = self.client.get(
reverse('api:elements-parents', kwargs={'pk': str(self.common_children.id)}),
data={'type': self.classified.slug, 'best_class': str(self.text.id)}
@@ -419,7 +419,7 @@ class TestClasses(FixtureAPITestCase):
self.populate_classified_elements()
child = Element.objects.filter(type=self.classified.id).first()
child.classifications.all().filter(confidence=.7).update(state=ClassificationState.Validated)
- with self.assertNumQueries(5):
+ with self.assertNumQueries(6):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': str(self.parent.id)}),
data={'type': self.classified.slug, 'best_class': str(self.text.id)}
@@ -449,7 +449,7 @@ class TestClasses(FixtureAPITestCase):
self.populate_classified_elements()
self.assertEqual(Classification.objects.filter(high_confidence=True).count(), 24)
self.assertEqual(Classification.objects.filter(high_confidence=True).distinct('element_id').count(), 12)
- with self.assertNumQueries(5):
+ with self.assertNumQueries(6):
response = self.client.get(
reverse('api:elements-parents', kwargs={'pk': str(self.common_children.id)}),
data={'type': self.classified.slug, 'best_class': str(self.cover.id)}
@@ -465,7 +465,7 @@ class TestClasses(FixtureAPITestCase):
self.populate_classified_elements()
self.assertEqual(Classification.objects.filter(high_confidence=True).count(), 24)
self.assertEqual(Classification.objects.filter(high_confidence=True).distinct('element_id').count(), 12)
- with self.assertNumQueries(5):
+ with self.assertNumQueries(6):
response = self.client.get(
reverse('api:elements-children', kwargs={'pk': str(self.parent.id)}),
data={'type': self.classified.slug, 'best_class': str(self.cover.id)}
diff --git a/arkindex/documents/tests/test_destroy_elements.py b/arkindex/documents/tests/test_destroy_elements.py
index 18e0b5b207e20d37a01fd7ba60f6d7162ee70a36..1d1f69a3936333332f331a0ebefa43cd88824a73 100644
--- a/arkindex/documents/tests/test_destroy_elements.py
+++ b/arkindex/documents/tests/test_destroy_elements.py
@@ -1,3 +1,5 @@
+from unittest.mock import patch
+
from django.urls import reverse
from rest_framework import status
@@ -13,6 +15,7 @@ class TestDestroyElements(FixtureAPITestCase):
super().setUpTestData()
cls.volume_type = cls.corpus.types.get(slug='volume')
cls.vol = cls.corpus.elements.get(name='Volume 1')
+ cls.surface = cls.corpus.elements.get(name='Surface A')
cls.private_corpus = Corpus.objects.create(name='private', public=False)
def test_element_destroy_verified_user(self):
@@ -160,3 +163,258 @@ class TestDestroyElements(FixtureAPITestCase):
Element.objects.filter(id=elements['A'].id).trash()
self.assertFalse(Element.objects.filter(id__in=[e.id for e in elements.values()]).exists())
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_corpus_elements_requires_login(self, delay_mock):
+ with self.assertNumQueries(0):
+ response = self.client.delete(reverse('api:corpus-elements', kwargs={'corpus': self.corpus.id}))
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+ self.assertFalse(delay_mock.called)
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_corpus_elements_requires_writable(self, delay_mock):
+ self.user.corpus_right.filter(corpus=self.corpus).update(can_write=False, can_admin=False)
+ self.client.force_login(self.user)
+
+ with self.assertNumQueries(4):
+ response = self.client.delete(reverse('api:corpus-elements', kwargs={'corpus': self.corpus.id}))
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+ self.assertFalse(delay_mock.called)
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_corpus_elements_empty(self, delay_mock):
+ self.client.force_login(self.user)
+ self.assertFalse(self.corpus.elements.filter(name='blablablabla').exists())
+
+ with self.assertNumQueries(5):
+ response = self.client.delete(
+ reverse('api:corpus-elements', kwargs={'corpus': self.corpus.id}),
+ QUERY_STRING='name=blablablabla',
+ )
+ self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+ self.assertFalse(delay_mock.called)
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_corpus_elements(self, delay_mock):
+ self.client.force_login(self.user)
+ with self.assertNumQueries(5):
+ response = self.client.delete(reverse('api:corpus-elements', kwargs={'corpus': self.corpus.id}))
+ self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
+
+ self.assertEqual(delay_mock.call_count, 1)
+ args, kwargs = delay_mock.call_args
+ self.assertEqual(len(args), 0)
+ self.assertCountEqual(list(kwargs.pop('queryset')), list(self.corpus.elements.all()))
+ self.assertDictEqual(kwargs, {'delete_children': True})
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_corpus_elements_delete_children(self, delay_mock):
+ self.client.force_login(self.user)
+ with self.assertNumQueries(5):
+ response = self.client.delete(
+ reverse('api:corpus-elements', kwargs={'corpus': self.corpus.id}),
+ QUERY_STRING='delete_children=false'
+ )
+ self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
+
+ self.assertEqual(delay_mock.call_count, 1)
+ args, kwargs = delay_mock.call_args
+ self.assertEqual(len(args), 0)
+ self.assertCountEqual(list(kwargs.pop('queryset')), list(self.corpus.elements.all()))
+ self.assertDictEqual(kwargs, {'delete_children': False})
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_corpus_elements_rejected_filters(self, delay_mock):
+ self.client.force_login(self.user)
+ with self.assertNumQueries(2):
+ response = self.client.delete(
+ reverse('api:corpus-elements', kwargs={'corpus': self.corpus.id}),
+ QUERY_STRING='with_best_classes=True&with_has_children=True&best_class=True',
+ HTTP_IF_MODIFIED_SINCE='Thu, 02 Apr 2099 13:37:42 GMT',
+ )
+ self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+ self.assertDictEqual(response.json(), {
+ 'If-Modified-Since': ['This parameter is not allowed in DELETE requests.'],
+ 'best_class': ['This parameter is not allowed in DELETE requests.'],
+ 'with_best_classes': ['This parameter is not allowed in DELETE requests.'],
+ 'with_has_children': ['This parameter is not allowed in DELETE requests.'],
+ })
+
+ self.assertFalse(delay_mock.called)
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_element_children_requires_login(self, delay_mock):
+ with self.assertNumQueries(0):
+ response = self.client.delete(reverse('api:elements-children', kwargs={'pk': self.vol.id}))
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+ self.assertFalse(delay_mock.called)
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_element_children_requires_writable(self, delay_mock):
+ self.user.corpus_right.filter(corpus=self.corpus).update(can_write=False, can_admin=False)
+ self.client.force_login(self.user)
+
+ with self.assertNumQueries(3):
+ response = self.client.delete(reverse('api:elements-children', kwargs={'pk': self.vol.id}))
+ self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+ self.assertFalse(delay_mock.called)
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_element_children_empty(self, delay_mock):
+ self.client.force_login(self.user)
+ element = self.corpus.elements.create(type=self.volume_type, name='Lonely element')
+
+ with self.assertNumQueries(4):
+ response = self.client.delete(reverse('api:elements-children', kwargs={'pk': element.id}))
+ self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+ self.assertFalse(delay_mock.called)
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_element_children(self, delay_mock):
+ self.client.force_login(self.user)
+ with self.assertNumQueries(4):
+ response = self.client.delete(reverse('api:elements-children', kwargs={'pk': self.vol.id}))
+ self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
+
+ self.assertEqual(delay_mock.call_count, 1)
+ args, kwargs = delay_mock.call_args
+ self.assertEqual(len(args), 0)
+ self.assertCountEqual(
+ list(kwargs.pop('queryset')),
+ # Direct children of the volume
+ list(Element.objects.get_descending(self.vol.id).filter(paths__path__last=self.vol.id)),
+ )
+ self.assertDictEqual(kwargs, {'delete_children': True})
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_element_children_delete_children(self, delay_mock):
+ self.client.force_login(self.user)
+ with self.assertNumQueries(4):
+ response = self.client.delete(
+ reverse('api:elements-children', kwargs={'pk': self.vol.id}),
+ QUERY_STRING='delete_children=false'
+ )
+ self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
+
+ self.assertEqual(delay_mock.call_count, 1)
+ args, kwargs = delay_mock.call_args
+ self.assertEqual(len(args), 0)
+ self.assertCountEqual(
+ list(kwargs.pop('queryset')),
+ # Direct children of the volume
+ list(Element.objects.get_descending(self.vol.id).filter(paths__path__last=self.vol.id)),
+ )
+ self.assertDictEqual(kwargs, {'delete_children': False})
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_element_children_rejected_filters(self, delay_mock):
+ self.client.force_login(self.user)
+ with self.assertNumQueries(2):
+ response = self.client.delete(
+ reverse('api:elements-children', kwargs={'pk': self.vol.id}),
+ QUERY_STRING='with_best_classes=True&with_has_children=True&best_class=True',
+ HTTP_IF_MODIFIED_SINCE='Thu, 02 Apr 2099 13:37:42 GMT',
+ )
+ self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+ self.assertDictEqual(response.json(), {
+ 'If-Modified-Since': ['This parameter is not allowed in DELETE requests.'],
+ 'best_class': ['This parameter is not allowed in DELETE requests.'],
+ 'with_best_classes': ['This parameter is not allowed in DELETE requests.'],
+ 'with_has_children': ['This parameter is not allowed in DELETE requests.'],
+ })
+
+ self.assertFalse(delay_mock.called)
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_element_parents_requires_login(self, delay_mock):
+ with self.assertNumQueries(0):
+ response = self.client.delete(reverse('api:elements-parents', kwargs={'pk': self.surface.id}))
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+ self.assertFalse(delay_mock.called)
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_element_parents_requires_writable(self, delay_mock):
+ self.user.corpus_right.filter(corpus=self.corpus).update(can_write=False, can_admin=False)
+ self.client.force_login(self.user)
+
+ with self.assertNumQueries(3):
+ response = self.client.delete(reverse('api:elements-parents', kwargs={'pk': self.surface.id}))
+ self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+ self.assertFalse(delay_mock.called)
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_element_parents_empty(self, delay_mock):
+ self.client.force_login(self.user)
+ element = self.corpus.elements.create(type=self.volume_type, name='Lonely element')
+
+ with self.assertNumQueries(4):
+ response = self.client.delete(reverse('api:elements-parents', kwargs={'pk': element.id}))
+ self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+ self.assertFalse(delay_mock.called)
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_element_parents(self, delay_mock):
+ self.client.force_login(self.user)
+ with self.assertNumQueries(4):
+ response = self.client.delete(reverse('api:elements-parents', kwargs={'pk': self.surface.id}))
+ self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
+
+ self.assertEqual(delay_mock.call_count, 1)
+ args, kwargs = delay_mock.call_args
+ self.assertEqual(len(args), 0)
+ self.assertCountEqual(
+ list(kwargs.pop('queryset')),
+ # Direct parents of the surface
+ list(Element.objects.get_ascending(self.surface.id, recursive=False)),
+ )
+ self.assertDictEqual(kwargs, {'delete_children': True})
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_element_parents_delete_parents(self, delay_mock):
+ self.client.force_login(self.user)
+ with self.assertNumQueries(4):
+ response = self.client.delete(
+ reverse('api:elements-parents', kwargs={'pk': self.surface.id}),
+ QUERY_STRING='delete_children=false'
+ )
+ self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
+
+ self.assertEqual(delay_mock.call_count, 1)
+ args, kwargs = delay_mock.call_args
+ self.assertEqual(len(args), 0)
+ self.assertCountEqual(
+ list(kwargs.pop('queryset')),
+ # Direct parents of the surface
+ list(Element.objects.get_ascending(self.surface.id, recursive=False)),
+ )
+ self.assertDictEqual(kwargs, {'delete_children': False})
+
+ @patch('arkindex.project.triggers.tasks.element_trash.delay')
+ def test_destroy_element_parents_rejected_filters(self, delay_mock):
+ self.client.force_login(self.user)
+ with self.assertNumQueries(2):
+ response = self.client.delete(
+ reverse('api:elements-parents', kwargs={'pk': self.surface.id}),
+ QUERY_STRING='with_best_classes=True&with_has_children=True&best_class=True',
+ HTTP_IF_MODIFIED_SINCE='Thu, 02 Apr 2099 13:37:42 GMT',
+ )
+ self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+ self.assertDictEqual(response.json(), {
+ 'If-Modified-Since': ['This parameter is not allowed in DELETE requests.'],
+ 'best_class': ['This parameter is not allowed in DELETE requests.'],
+ 'with_best_classes': ['This parameter is not allowed in DELETE requests.'],
+ 'with_has_children': ['This parameter is not allowed in DELETE requests.'],
+ })
+
+ self.assertFalse(delay_mock.called)
diff --git a/arkindex/documents/tests/test_parents_elements.py b/arkindex/documents/tests/test_parents_elements.py
index 6ad84a8a1861ca99e505cedc2ff0cfdd8c989f88..3e65fb504b5b889e4c912ab7067477f25b2439bf 100644
--- a/arkindex/documents/tests/test_parents_elements.py
+++ b/arkindex/documents/tests/test_parents_elements.py
@@ -76,7 +76,7 @@ class TestParentsElements(FixtureAPITestCase):
"""
self.corpus.elements.filter(name__contains='Volume 1').update(worker_version=self.worker_version)
- with self.assertNumQueries(6):
+ with self.assertNumQueries(7):
response = self.client.get(
reverse('api:elements-parents', kwargs={'pk': str(self.page.id)}),
data={'worker_version': str(self.worker_version.id)},
@@ -89,7 +89,7 @@ class TestParentsElements(FixtureAPITestCase):
)
def test_parents_worker_version_validation(self):
- with self.assertNumQueries(0):
+ with self.assertNumQueries(1):
response = self.client.get(
reverse('api:elements-parents', kwargs={'pk': str(self.page.id)}),
data={'worker_version': 'blah'},
@@ -97,7 +97,7 @@ class TestParentsElements(FixtureAPITestCase):
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.json(), {'worker_version': ['Invalid UUID']})
- with self.assertNumQueries(1):
+ with self.assertNumQueries(2):
response = self.client.get(
reverse('api:elements-parents', kwargs={'pk': str(self.page.id)}),
data={'worker_version': uuid.uuid4()},
@@ -107,7 +107,7 @@ class TestParentsElements(FixtureAPITestCase):
def test_parents_with_has_children(self):
surface = Element.objects.get(name='Surface A')
- with self.assertNumQueries(7):
+ with self.assertNumQueries(8):
response = self.client.get(
reverse('api:elements-parents', kwargs={'pk': str(surface.id)}),
data={'recursive': True, 'with_has_children': True},
@@ -122,7 +122,7 @@ class TestParentsElements(FixtureAPITestCase):
)
def test_parents_modified_since_bad_format(self):
- with self.assertNumQueries(0):
+ with self.assertNumQueries(1):
response = self.client.get(
reverse('api:elements-parents', kwargs={'pk': str(self.page.id)}),
HTTP_IF_MODIFIED_SINCE='AAAAAAAAAAAAAAAAA',
@@ -131,7 +131,7 @@ class TestParentsElements(FixtureAPITestCase):
self.assertDictEqual(response.json(), {'If-Modified-Since': ['Bad date format']})
def test_parents_modified_since_not_modified(self):
- with self.assertNumQueries(1):
+ with self.assertNumQueries(2):
response = self.client.get(
reverse('api:elements-parents', kwargs={'pk': str(self.page.id)}),
HTTP_IF_MODIFIED_SINCE='Thu, 02 Apr 2099 13:37:42 GMT',
@@ -140,7 +140,7 @@ class TestParentsElements(FixtureAPITestCase):
def test_parents_modified_since(self):
self.corpus.elements.filter(name='Volume 1').update(updated='2099-04-02T13:37:43Z')
- with self.assertNumQueries(6):
+ with self.assertNumQueries(7):
response = self.client.get(
reverse('api:elements-parents', kwargs={'pk': str(self.page.id)}),
HTTP_IF_MODIFIED_SINCE='Thu, 02 Apr 2099 13:37:42 GMT',
diff --git a/arkindex/project/openapi/patch.yml b/arkindex/project/openapi/patch.yml
index 66cc7c59dabebda8d6f4c77baa00f05f7ef64622..60a825fc83ea4b2eaead8a51ff820cd21f24d429 100644
--- a/arkindex/project/openapi/patch.yml
+++ b/arkindex/project/openapi/patch.yml
@@ -49,6 +49,10 @@ paths:
security: []
post:
description: Create a new corpus
+ /api/v1/corpus/{corpus}/elements/:
+ delete:
+ operationId: DestroyElements
+ description: Delete elements in bulk
/api/v1/corpus/{id}/:
get:
description: Retrieve a single corpus
@@ -116,6 +120,14 @@ paths:
# Will need https://gitlab.com/arkindex/backend/-/issues/86 to be removed
operationId: DestroyElementMLResults
description: Delete machine learning results on an element and its direct children.
+ /api/v1/elements/{id}/children/:
+ delete:
+ operationId: DestroyElementChildren
+ description: Delete child elements in bulk
+ /api/v1/elements/{id}/parents/:
+ delete:
+ operationId: DestroyElementParents
+ description: Delete parent elements in bulk
/api/v1/elements/selection/:
delete:
operationId: RemoveSelection
diff --git a/arkindex/project/openapi/schemas.py b/arkindex/project/openapi/schemas.py
index ae8c9c3e0c5d816fa4a92da39f49f8d10d007def..a813b80fb83a64938682db47bbe6cc834a1238b3 100644
--- a/arkindex/project/openapi/schemas.py
+++ b/arkindex/project/openapi/schemas.py
@@ -82,8 +82,13 @@ class AutoSchema(BaseAutoSchema):
# Avoid removing the parameter overrides with .pop()
overrides = self.view.openapi_overrides.copy()
if 'parameters' in overrides:
+ # Filter parameters by HTTP method
+ allowed_parameters = [
+ parameter for parameter in overrides.pop('parameters')
+ if 'x-methods' not in parameter or method.lower() in parameter['x-methods']
+ ]
# Append parameters instead of replacing
- operation.setdefault('parameters', []).extend(overrides.pop('parameters'))
+ operation.setdefault('parameters', []).extend(allowed_parameters)
operation.update(overrides)
return operation
diff --git a/arkindex/project/tests/openapi/test_schemas.py b/arkindex/project/tests/openapi/test_schemas.py
index dc34385c80e49ac577e62072ecac45cba885ed9a..79ef30012f40e174449906840c9c348eb302e490 100644
--- a/arkindex/project/tests/openapi/test_schemas.py
+++ b/arkindex/project/tests/openapi/test_schemas.py
@@ -155,6 +155,107 @@ class TestAutoSchema(TestCase):
}
)
+ def test_overrides_method_filter(self):
+ """
+ Test the `x-methods` filter for parameters on openapi_overrides
+ """
+
+ class ThingView(APIView):
+ action = 'Retrieve'
+ openapi_overrides = {
+ 'parameters': [
+ {
+ 'description': 'Some extra parameter',
+ 'in': 'query',
+ 'name': 'something_get',
+ 'required': False,
+ 'schema': {'type': 'integer'},
+ 'x-methods': ['get'],
+ },
+ {
+ 'description': 'Some extra parameter',
+ 'in': 'query',
+ 'name': 'something_post',
+ 'required': False,
+ 'schema': {'type': 'integer'},
+ 'x-methods': ['post', 'delete'],
+ },
+ {
+ 'description': 'Some extra parameter',
+ 'in': 'query',
+ 'name': 'something',
+ 'required': False,
+ 'schema': {'type': 'integer'},
+ }
+ ],
+ }
+
+ def get(self, *args, **kwargs):
+ pass
+
+ def post(self, *args, **kwargs):
+ pass
+
+ inspector = AutoSchema()
+ inspector.view = create_view(ThingView, 'GET', create_request('/test/{id}/'))
+ self.assertListEqual(
+ inspector.get_operation('/test/{id}/', 'GET')['parameters'],
+ [
+ {
+ 'description': '',
+ 'in': 'path',
+ 'name': 'id',
+ 'required': True,
+ 'schema': {'type': 'string'},
+ },
+ {
+ 'description': 'Some extra parameter',
+ 'in': 'query',
+ 'name': 'something_get',
+ 'required': False,
+ 'schema': {'type': 'integer'},
+ 'x-methods': ['get'],
+ },
+ {
+ 'description': 'Some extra parameter',
+ 'in': 'query',
+ 'name': 'something',
+ 'required': False,
+ 'schema': {'type': 'integer'},
+ }
+ ],
+ )
+
+ inspector = AutoSchema()
+ inspector.view = create_view(ThingView, 'POST', create_request('/test/{id}/'))
+ self.assertListEqual(
+ inspector.get_operation('/test/{id}/', 'POST')['parameters'],
+ [
+ {
+ 'description': '',
+ 'in': 'path',
+ 'name': 'id',
+ 'required': True,
+ 'schema': {'type': 'string'},
+ },
+ {
+ 'description': 'Some extra parameter',
+ 'in': 'query',
+ 'name': 'something_post',
+ 'required': False,
+ 'schema': {'type': 'integer'},
+ 'x-methods': ['post', 'delete'],
+ },
+ {
+ 'description': 'Some extra parameter',
+ 'in': 'query',
+ 'name': 'something',
+ 'required': False,
+ 'schema': {'type': 'integer'},
+ }
+ ],
+ )
+
def test_bugfix_list_uppercase(self):
"""
Test list API views have title-cased endpoint names
diff --git a/arkindex/project/triggers.py b/arkindex/project/triggers.py
index a975001155e581fb3ceb21c9021ae4b243d2c440..b95538b071a0ec571b6beaa5f7ee8eafb19cce8e 100644
--- a/arkindex/project/triggers.py
+++ b/arkindex/project/triggers.py
@@ -7,6 +7,7 @@ from uuid import UUID
from django.conf import settings
from arkindex.documents import tasks
+from arkindex.documents.managers import ElementQuerySet
from arkindex.documents.models import Corpus, Element, Entity
@@ -102,3 +103,11 @@ def corpus_delete(corpus: Union[Corpus, UUID, str]) -> None:
corpus_id = str(corpus)
tasks.corpus_delete.delay(corpus_id=corpus_id)
+
+
+def element_trash(queryset: ElementQuerySet, delete_children: bool = True) -> None:
+ """
+ Run ElementQuerySet.trash to delete a batch of elements.
+ """
+ assert isinstance(queryset, ElementQuerySet), 'Only Element querysets can be trashed'
+ tasks.element_trash.delay(queryset=queryset, delete_children=delete_children)