From 1faa63f56c29f4d4bb67a41c486de0f2b5c8c2f8 Mon Sep 17 00:00:00 2001 From: Roberto Rosario Date: Tue, 15 Aug 2017 00:17:40 -0400 Subject: [PATCH] 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 --- mayan/apps/documents/api_views.py | 30 ++++++- mayan/apps/documents/models.py | 15 +++- mayan/apps/documents/tests/test_api.py | 78 ++++++++++--------- mayan/apps/documents/tests/test_views.py | 50 +++++++++--- .../documents/views/document_version_views.py | 18 ++--- mayan/apps/documents/views/document_views.py | 1 - 6 files changed, 130 insertions(+), 62 deletions(-) diff --git a/mayan/apps/documents/api_views.py b/mayan/apps/documents/api_views.py index d850f8e211..10640e26fc 100644 --- a/mayan/apps/documents/api_views.py +++ b/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 diff --git a/mayan/apps/documents/models.py b/mayan/apps/documents/models.py index 1f3f487e3c..1baed686d3 100644 --- a/mayan/apps/documents/models.py +++ b/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,10 +410,16 @@ class DocumentVersion(models.Model): def get_absolute_url(self): return reverse('documents:document_version_view', args=(self.pk,)) - def get_rendered_string(self): - return Template( - '{{ instance.document }} - {{ instance.timestamp }}' - ).render(context=Context({'instance': 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})) def get_rendered_timestamp(self): return Template('{{ instance.timestamp }}').render( diff --git a/mayan/apps/documents/tests/test_api.py b/mayan/apps/documents/tests/test_api.py index b74ba8f0d8..b58e398cef 100644 --- a/mayan/apps/documents/tests/test_api.py +++ b/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( diff --git a/mayan/apps/documents/tests/test_views.py b/mayan/apps/documents/tests/test_views.py index f0b62bcd5d..3db53590be 100644 --- a/mayan/apps/documents/tests/test_views.py +++ b/mayan/apps/documents/tests/test_views.py @@ -286,27 +286,54 @@ 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) @@ -314,10 +341,11 @@ class DocumentsViewsTestCase(GenericDocumentViewTestCase): 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): diff --git a/mayan/apps/documents/views/document_version_views.py b/mayan/apps/documents/views/document_version_views.py index 4c72d36012..f40b363ebd 100644 --- a/mayan/apps/documents/views/document_version_views.py +++ b/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): diff --git a/mayan/apps/documents/views/document_views.py b/mayan/apps/documents/views/document_views.py index ac1cc3ea14..3329b19aeb 100644 --- a/mayan/apps/documents/views/document_views.py +++ b/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