Delete documents in form if file is no longer in redis. Add test for getting file by key

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):

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
         }

edivorce/apps/core/urls.py

@@ -9,7 +9,7 @@ urlpatterns = [
     url(r'^api/documents/$', api.DocumentCreateView.as_view(), name='documents'),
     path('api/documents/<doc_type>/<int:party_code>/', api.DocumentMetaDataView.as_view(), name='documents-meta'),
     path('api/documents/<doc_type>/<int:party_code>/<int:size>/<filename>', api.DocumentView.as_view(), name='document'),
-    path('api/documents/<file_key>/', api.get_document_file_by_key, name='document_by_key'),
+    path('api/documents/<file_key>/', api.get_document_file_by_key, name='file_by_key'),
 
     # url(r'^login/headers$', system.headers),
 

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")