Move extension preservetation code to the model. Improve document,

version and API views to include proper mimetype and encoding.
Update respective tests. GitLab issue #415.

Signed-off-by: Roberto Rosario <roberto.rosario.gonzalez@gmail.com>

mayan/apps/documents/api_views.py

@@ -4,7 +4,6 @@ import logging
 
 from django.http import HttpResponse
 from django.shortcuts import get_object_or_404
-from django.utils.encoding import force_text
 
 from django_downloadview import DownloadMixin, VirtualFile
 from rest_framework import generics, status
@@ -109,10 +108,16 @@ class APIDocumentDownloadView(DownloadMixin, generics.RetrieveAPIView):
     permission_classes = (MayanPermission,)
     queryset = Document.objects.all()
 
+    def get_encoding(self):
+        return self.get_object().latest_version.encoding
+
     def get_file(self):
         instance = self.get_object()
         return VirtualFile(instance.latest_version.file, name=instance.label)
 
+    def get_mimetype(self):
+        return self.get_object().latest_version.mimetype
+
     def get_serializer_class(self):
         return None
 
@@ -186,6 +191,9 @@ class APIDocumentVersionDownloadView(DownloadMixin, generics.RetrieveAPIView):
             - name: pk
               paramType: path
               type: number
+            - name: preserve_extension
+              paramType: query
+              type: boolean
     """
     lookup_url_kwarg = 'version_pk'
 
@@ -197,9 +205,27 @@ class APIDocumentVersionDownloadView(DownloadMixin, generics.RetrieveAPIView):
         )
         return document
 
+    def get_encoding(self):
+        return self.get_object().encoding
+
     def get_file(self):
+        preserve_extension = self.request.GET.get(
+            'preserve_extension', self.request.POST.get(
+                'preserve_extension', False
+            )
+        )
+
+        preserve_extension = preserve_extension == 'true' or preserve_extension == 'True'
+
         instance = self.get_object()
-        return VirtualFile(instance.file, name=force_text(instance))
+        return VirtualFile(
+            instance.file, name=instance.get_rendered_string(
+                preserve_extension=preserve_extension
+            )
+        )
+
+    def get_mimetype(self):
+        return self.get_object().mimetype
 
     def get_serializer_class(self):
         return None

mayan/apps/documents/models.py

@@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals
 
 import hashlib
 import logging
+import os
 import uuid
 
 from django.conf import settings
@@ -409,7 +410,13 @@ class DocumentVersion(models.Model):
     def get_absolute_url(self):
         return reverse('documents:document_version_view', args=(self.pk,))
 
-    def get_rendered_string(self):
+    def get_rendered_string(self, preserve_extension=False):
+        if preserve_extension:
+            filename, extension = os.path.splitext(self.document.label)
+            return '{} ({}){}'.format(
+                filename, self.get_rendered_timestamp(), extension
+            )
+        else:
             return Template(
                 '{{ instance.document }} - {{ instance.timestamp }}'
             ).render(context=Context({'instance': self}))

mayan/apps/documents/tests/test_api.py

@@ -119,12 +119,16 @@ class DocumentAPITestCase(BaseAPITestCase):
         self.document_type.delete()
         super(DocumentAPITestCase, self).tearDown()
 
-    def _upload_document(self):
+    def _create_document(self):
         with open(TEST_SMALL_DOCUMENT_PATH) as file_object:
             self.document = self.document_type.new_document(
                 file_object=file_object,
+                label=TEST_SMALL_DOCUMENT_FILENAME
             )
 
+        # For compatibility
+        return self.document
+
     def test_document_upload(self):
         with open(TEST_DOCUMENT_PATH) as file_descriptor:
             response = self.client.post(
@@ -160,10 +164,7 @@ class DocumentAPITestCase(BaseAPITestCase):
         self.assertEqual(document.page_count, 47)
 
     def test_document_new_version_upload(self):
-        with open(TEST_SMALL_DOCUMENT_PATH) as file_object:
-            document = self.document_type.new_document(
-                file_object=file_object,
-            )
+        document = self._create_document()
 
         # Artifical delay since MySQL doesn't store microsecond data in
         # timestamps. Version timestamp is used to determine which version
@@ -193,10 +194,7 @@ class DocumentAPITestCase(BaseAPITestCase):
         self.assertEqual(document.page_count, 47)
 
     def test_document_version_revert(self):
-        with open(TEST_SMALL_DOCUMENT_PATH) as file_object:
-            document = self.document_type.new_document(
-                file_object=file_object,
-            )
+        document = self._create_document()
 
         # Needed by MySQL as milliseconds value is not store in timestamp field
         time.sleep(1)
@@ -220,10 +218,7 @@ class DocumentAPITestCase(BaseAPITestCase):
         self.assertEqual(document.versions.first(), document.latest_version)
 
     def test_document_version_list(self):
-        with open(TEST_SMALL_DOCUMENT_PATH) as file_object:
-            document = self.document_type.new_document(
-                file_object=file_object,
-            )
+        document = self._create_document()
 
         # Needed by MySQL as milliseconds value is not store in timestamp field
         time.sleep(1)
@@ -248,10 +243,7 @@ class DocumentAPITestCase(BaseAPITestCase):
         )
 
     def test_document_download(self):
-        with open(TEST_SMALL_DOCUMENT_PATH) as file_object:
-            document = self.document_type.new_document(
-                file_object=file_object,
-            )
+        document = self._create_document()
 
         response = self.client.get(
             reverse(
@@ -267,10 +259,7 @@ class DocumentAPITestCase(BaseAPITestCase):
             )
 
     def test_document_version_download(self):
-        with open(TEST_SMALL_DOCUMENT_PATH) as file_object:
-            document = self.document_type.new_document(
-                file_object=file_object,
-            )
+        document = self._create_document()
 
         latest_version = document.latest_version
         response = self.client.get(
@@ -283,14 +272,33 @@ class DocumentAPITestCase(BaseAPITestCase):
         with latest_version.open() as file_object:
             assert_download_response(
                 self, response, content=file_object.read(),
-                basename='{} - {}'.format(
-                    TEST_SMALL_DOCUMENT_FILENAME,
-                    latest_version.timestamp
-                ), mime_type='application/octet-stream; charset=utf-8'
+                basename=force_text(latest_version),
+                mime_type='{}; charset=utf-8'.format(document.file_mimetype)
+            )
+
+    def test_document_version_download_preserve_extension(self):
+        document = self._create_document()
+
+        latest_version = document.latest_version
+        response = self.client.get(
+            reverse(
+                'rest_api:documentversion-download',
+                args=(document.pk, latest_version.pk,)
+            ), data={'preserve_extension': True}
+        )
+
+        with latest_version.open() as file_object:
+            assert_download_response(
+                self, response, content=file_object.read(),
+                basename=latest_version.get_rendered_string(
+                    preserve_extension=True
+                ), mime_type='{}; charset=utf-8'.format(
+                    document.file_mimetype
+                )
             )
 
     def test_document_version_edit_via_patch(self):
-        self._upload_document()
+        self._create_document()
         response = self.client.patch(
             reverse(
                 'rest_api:documentversion-detail',
@@ -307,7 +315,7 @@ class DocumentAPITestCase(BaseAPITestCase):
         )
 
     def test_document_version_edit_via_put(self):
-        self._upload_document()
+        self._create_document()
         response = self.client.put(
             reverse(
                 'rest_api:documentversion-detail',
@@ -324,7 +332,7 @@ class DocumentAPITestCase(BaseAPITestCase):
         )
 
     def test_document_comment_edit_via_patch(self):
-        self._upload_document()
+        self._create_document()
         response = self.client.patch(
             reverse(
                 'rest_api:document-detail',
@@ -340,7 +348,7 @@ class DocumentAPITestCase(BaseAPITestCase):
         )
 
     def test_document_comment_edit_via_put(self):
-        self._upload_document()
+        self._create_document()
         response = self.client.put(
             reverse(
                 'rest_api:document-detail',
@@ -377,7 +385,7 @@ class TrashedDocumentAPITestCase(BaseAPITestCase):
         self.document_type.delete()
         super(TrashedDocumentAPITestCase, self).tearDown()
 
-    def _upload_document(self):
+    def _create_document(self):
         with open(TEST_SMALL_DOCUMENT_PATH) as file_object:
             document = self.document_type.new_document(
                 file_object=file_object,
@@ -386,7 +394,7 @@ class TrashedDocumentAPITestCase(BaseAPITestCase):
         return document
 
     def test_document_move_to_trash(self):
-        document = self._upload_document()
+        document = self._create_document()
 
         self.client.delete(
             reverse('rest_api:document-detail', args=(document.pk,))
@@ -396,7 +404,7 @@ class TrashedDocumentAPITestCase(BaseAPITestCase):
         self.assertEqual(Document.trash.count(), 1)
 
     def test_trashed_document_delete_from_trash(self):
-        document = self._upload_document()
+        document = self._create_document()
         document.delete()
 
         self.assertEqual(Document.objects.count(), 0)
@@ -409,7 +417,7 @@ class TrashedDocumentAPITestCase(BaseAPITestCase):
         self.assertEqual(Document.trash.count(), 0)
 
     def test_trashed_document_detail_view(self):
-        document = self._upload_document()
+        document = self._create_document()
         document.delete()
 
         response = self.client.get(
@@ -419,7 +427,7 @@ class TrashedDocumentAPITestCase(BaseAPITestCase):
         self.assertEqual(response.data['uuid'], force_text(document.uuid))
 
     def test_trashed_document_list_view(self):
-        document = self._upload_document()
+        document = self._create_document()
         document.delete()
 
         response = self.client.get(
@@ -429,7 +437,7 @@ class TrashedDocumentAPITestCase(BaseAPITestCase):
         self.assertEqual(response.data['results'][0]['uuid'], force_text(document.uuid))
 
     def test_trashed_document_restore(self):
-        document = self._upload_document()
+        document = self._create_document()
         document.delete()
 
         self.client.post(

mayan/apps/documents/tests/test_views.py

@@ -286,38 +286,66 @@ class DocumentsViewsTestCase(GenericDocumentViewTestCase):
                 mime_type=self.document.file_mimetype
             )
 
-    def test_document_version_download_view_no_permission(self):
-        response = self.get(
+    def _request_document_version_download(self, data=None):
+        data = data or {}
+        return self.get(
             'documents:document_version_download', args=(
                 self.document.latest_version.pk,
+            ), data=data
         )
-        )
+
+    def test_document_version_download_view_no_permission(self):
+        response = self._request_document_version_download()
 
         self.assertEqual(response.status_code, 403)
 
     def test_document_version_download_view_with_permission(self):
         # Set the expected_content_type for
         # common.tests.mixins.ContentTypeCheckMixin
-        self.expected_content_type = 'application/octet-stream; charset=utf-8'
+        self.expected_content_type = '{}; charset=utf-8'.format(
+            self.document.latest_version.mimetype
+        )
 
         self.grant_access(
             obj=self.document, permission=permission_document_download
         )
-        response = self.get(
-            'documents:document_version_download', args=(
-                self.document.latest_version.pk,
+        response = self._request_document_version_download()
+
+        self.assertEqual(response.status_code, 200)
+
+        with self.document.open() as file_object:
+            self.assert_download_response(
+                response, content=file_object.read(),
+                basename=force_text(self.document.latest_version),
+                mime_type='{}; charset=utf-8'.format(
+                    self.document.latest_version.mimetype
                 )
             )
 
+    def test_document_version_download_preserve_extension_view_with_permission(self):
+        # Set the expected_content_type for
+        # common.tests.mixins.ContentTypeCheckMixin
+        self.expected_content_type = '{}; charset=utf-8'.format(
+            self.document.latest_version.mimetype
+        )
+
+        self.grant_access(
+            obj=self.document, permission=permission_document_download
+        )
+        response = self._request_document_version_download(
+            data={'preserve_extension': True}
+        )
+
         self.assertEqual(response.status_code, 200)
 
         with self.document.open() as file_object:
             self.assert_download_response(
                 response, content=file_object.read(),
-                basename='{} - {}'.format(
-                    TEST_SMALL_DOCUMENT_FILENAME,
-                    self.document.latest_version.timestamp
-                ), mime_type='application/octet-stream; charset=utf-8'
+                basename=self.document.latest_version.get_rendered_string(
+                    preserve_extension=True
+                ), mime_type='{}; charset=utf-8'.format(
+                    self.document.latest_version.mimetype
+                )
             )
 
     def test_document_update_page_count_view_no_permission(self):

mayan/apps/documents/views/document_version_views.py

@@ -1,11 +1,9 @@
 from __future__ import absolute_import, unicode_literals
 
 import logging
-import os
 
 from django.contrib import messages
 from django.shortcuts import get_object_or_404
-from django.utils.encoding import force_text
 from django.utils.translation import ugettext_lazy as _
 
 from acls.models import AccessControlList
@@ -121,6 +119,9 @@ class DocumentVersionDownloadView(DocumentDownloadView):
     def get_item_file(item):
         return item.file
 
+    def get_encoding(self):
+        return self.get_object().encoding
+
     def get_item_label(self, item):
         preserve_extension = self.request.GET.get(
             'preserve_extension', self.request.POST.get(
@@ -128,13 +129,12 @@ class DocumentVersionDownloadView(DocumentDownloadView):
             )
         )
 
-        if preserve_extension:
-            filename, extension = os.path.splitext(item.document.label)
-            return '{} ({}){}'.format(
-                filename, item.get_rendered_timestamp(), extension
-            )
-        else:
-            return force_text(item)
+        preserve_extension = preserve_extension == 'true' or preserve_extension == 'True'
+
+        return item.get_rendered_string(preserve_extension=preserve_extension)
+
+    def get_mimetype(self):
+        return self.get_object().mimetype
 
 
 class DocumentVersionView(SingleObjectDetailView):

mayan/apps/documents/views/document_views.py

@@ -7,7 +7,6 @@ from django.core.exceptions import PermissionDenied
 from django.http import HttpResponseRedirect
 from django.shortcuts import get_object_or_404
 from django.urls import reverse, reverse_lazy
-from django.utils.encoding import force_text
 from django.utils.http import urlencode
 from django.utils.translation import ugettext_lazy as _, ungettext