From 412a6aa4a287394ab12465b45b461116c2b23216 Mon Sep 17 00:00:00 2001 From: ariannedee Date: Thu, 1 Oct 2020 15:06:06 -0700 Subject: [PATCH] Delete documents in form if file is no longer in redis. Add test for getting file by key --- edivorce/apps/core/models.py | 11 ++++-- edivorce/apps/core/tests/test_api.py | 52 +++++++++++++++++++++------- edivorce/apps/core/urls.py | 2 +- edivorce/apps/core/views/api.py | 25 +++++++------ 4 files changed, 64 insertions(+), 26 deletions(-) diff --git a/edivorce/apps/core/models.py b/edivorce/apps/core/models.py index 145a2c06..292b4d0c 100644 --- a/edivorce/apps/core/models.py +++ b/edivorce/apps/core/models.py @@ -156,6 +156,9 @@ class Document(models.Model): self.filename = self.file.name if not self.size: self.size = self.file.size + if not self.sort_order: + num_docs = self.get_documents_in_form().count() + self.sort_order = num_docs + 1 super(Document, self).save(*args, **kwargs) @@ -174,16 +177,20 @@ class Document(models.Model): return reverse('document', kwargs={'filename': self.filename, 'doc_type': self.doc_type, 'party_code': self.party_code, 'size': self.size}) def update_sort_orders(self): - q = Document.objects.filter(bceid_user=self.bceid_user, doc_type=self.doc_type, party_code=self.party_code, sort_order__gt=self.sort_order) + q = self.get_documents_in_form().filter(sort_order__gt=self.sort_order) q.update(sort_order=F('sort_order') - 1) @staticmethod def get_file(file_key): - return redis.RedisStorage().get(file_key) + if redis.RedisStorage().exists(file_key): + return redis.RedisStorage().open(file_key) def file_exists(self): return redis.RedisStorage().exists(self.file.name) + def get_documents_in_form(self): + return Document.objects.filter(bceid_user=self.bceid_user, doc_type=self.doc_type, party_code=self.party_code) + class DontLog: def log_addition(self, *args): diff --git a/edivorce/apps/core/tests/test_api.py b/edivorce/apps/core/tests/test_api.py index 38ea8e5e..fd54738a 100644 --- a/edivorce/apps/core/tests/test_api.py +++ b/edivorce/apps/core/tests/test_api.py @@ -154,10 +154,8 @@ class APITest(APITestCase): def test_get_file(self): document = self._create_document() self.assertEqual(Document.objects.count(), 1) - url = reverse('document', kwargs={'doc_type': document.doc_type, - 'party_code': document.party_code, - 'filename': document.filename, - 'size': document.size}) + url = document.get_file_url() + response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) @@ -170,13 +168,45 @@ class APITest(APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.content, document.file.read()) + def test_get_file_missing_from_redis(self): + document = self._create_document() + self._create_document() + another_document = self._create_document(party_code=2) + + self.assertEqual(Document.objects.count(), 3) + url = document.get_file_url() + + self.client.force_authenticate(self.user) + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, document.file.read()) + + # Delete file from redis + document.file.delete() + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + self.assertEqual(Document.objects.count(), 1) + self.assertEqual(Document.objects.first(), another_document) + + def test_get_file_by_key(self): + document = self._create_document() + url = reverse('file_by_key', kwargs={'file_key': document.file.name}) + + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, document.file.read()) + + # Delete file from redis + document.file.delete() + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + def test_delete_document(self): document = self._create_document() self.assertEqual(Document.objects.count(), 1) - url = reverse('document', kwargs={'doc_type': document.doc_type, - 'party_code': document.party_code, - 'filename': document.filename, - 'size': document.size}) + url = document.get_file_url() + response = self.client.delete(url) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) self.assertEqual(Document.objects.count(), 1) @@ -193,10 +223,8 @@ class APITest(APITestCase): def test_update_document(self): document = self._create_document() - url = reverse('document', kwargs={'doc_type': document.doc_type, - 'party_code': document.party_code, - 'filename': document.filename, - 'size': document.size}) + url = document.get_file_url() + data = { 'rotation': 90 } diff --git a/edivorce/apps/core/urls.py b/edivorce/apps/core/urls.py index 3a3b232a..45a46689 100644 --- a/edivorce/apps/core/urls.py +++ b/edivorce/apps/core/urls.py @@ -9,7 +9,7 @@ urlpatterns = [ url(r'^api/documents/$', api.DocumentCreateView.as_view(), name='documents'), path('api/documents///', api.DocumentMetaDataView.as_view(), name='documents-meta'), path('api/documents////', api.DocumentView.as_view(), name='document'), - path('api/documents//', api.get_document_file_by_key, name='document_by_key'), + path('api/documents//', api.get_document_file_by_key, name='file_by_key'), # url(r'^login/headers$', system.headers), diff --git a/edivorce/apps/core/views/api.py b/edivorce/apps/core/views/api.py index b07854e1..29d23041 100644 --- a/edivorce/apps/core/views/api.py +++ b/edivorce/apps/core/views/api.py @@ -2,7 +2,7 @@ import re import graphene import graphene_django -from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseGone +from django.http import Http404, HttpResponse, HttpResponseGone, HttpResponseNotFound from graphql import GraphQLError from rest_framework import permissions, status from rest_framework.generics import CreateAPIView, ListAPIView, RetrieveUpdateDestroyAPIView @@ -82,10 +82,14 @@ class DocumentView(RetrieveUpdateDestroyAPIView): permission_classes = [permissions.IsAuthenticated] def get_object(self): - try: - return Document.objects.get(bceid_user=self.request.user, **self.kwargs) - except Document.DoesNotExist: + doc = Document.objects.filter(bceid_user=self.request.user, **self.kwargs).first() + if not doc: raise Http404("Document not found") + elif not doc.file_exists(): + doc.get_documents_in_form().delete() + raise Http404("Document no longer exists") + else: + return doc def retrieve(self, request, *args, **kwargs): """ Return the file instead of meta data """ @@ -103,11 +107,10 @@ class DocumentView(RetrieveUpdateDestroyAPIView): def get_document_file_by_key(request, file_key): file = Document.get_file(file_key) + if not file: + return HttpResponseNotFound() content_type = _content_type_from_filename(file.name) - try: - return HttpResponse(file, content_type=content_type) - except TypeError: - raise Http404("File not found") + return HttpResponse(file, content_type=content_type) def _content_type_from_filename(filename): @@ -147,7 +150,7 @@ class DocumentInput(graphene.InputObjectType): filename = graphene.String(required=True) size = graphene.Int(required=True) width = graphene.Int() - height = graphene.Int() + height = graphene.Int() rotation = graphene.Int() @@ -177,8 +180,8 @@ class UpdateMetadata(graphene.Mutation): try: doc = documents.get(filename=file['filename'], size=file['size']) doc.sort_order = i + 1 - doc.width = file.get('width', file.width) - doc.height = file.get('height', file.height) + doc.width = file.get('width', file.width) + doc.height = file.get('height', file.height) doc.rotation = file.get('rotation', file.rotation) if doc.rotation not in [0, 90, 180, 270]: raise GraphQLError(f"Invalid rotation {doc.rotation}, must be 0, 90, 180, 270")