From a7b31fc1716ee2256fa939324a77ffbf0ddd3490 Mon Sep 17 00:00:00 2001 From: Roberto Rosario Date: Thu, 12 Dec 2019 19:38:12 -0400 Subject: [PATCH] Refactor and implement download code natively - Use modified port of Django 2.2 FileResponse. - Remove Django DownloadView library. Signed-off-by: Roberto Rosario --- HISTORY.rst | 2 + mayan/apps/common/compat.py | 78 ++++++++++ mayan/apps/common/generics.py | 66 ++++++-- mayan/apps/common/mixins.py | 44 +++--- mayan/apps/common/tests/base.py | 8 +- mayan/apps/common/tests/mixins.py | 37 ++++- mayan/apps/django_gpg/tests/test_views.py | 12 +- mayan/apps/django_gpg/views.py | 8 +- .../apps/document_parsing/tests/test_views.py | 6 +- mayan/apps/document_parsing/views.py | 12 +- .../document_signatures/tests/test_views.py | 10 +- mayan/apps/document_signatures/views.py | 9 +- mayan/apps/documents/api_views.py | 30 ++-- mayan/apps/documents/forms/document_forms.py | 4 +- mayan/apps/documents/tests/mixins.py | 9 +- mayan/apps/documents/tests/test_api.py | 31 ++-- .../documents/tests/test_document_views.py | 61 ++++---- mayan/apps/documents/tests/test_events.py | 9 +- mayan/apps/documents/urls.py | 5 + .../documents/views/document_version_views.py | 33 +--- mayan/apps/documents/views/document_views.py | 142 ++++++------------ mayan/apps/ocr/tests/test_views.py | 4 +- mayan/apps/ocr/views.py | 12 +- removals.txt | 1 + 24 files changed, 355 insertions(+), 278 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 0a5752805d..81272c0ffd 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -20,6 +20,8 @@ - Add the ID and the URL to the checkout serializer. - Add BaseTransformationType metaclass in a way compatible with Python 2 and Python 3. +- Remove Django DownloadView library. Implement downloads natively + using modified port of Django 2.2 FileResponse. 3.3.4 (2019-12-09) ================== diff --git a/mayan/apps/common/compat.py b/mayan/apps/common/compat.py index fb0d6a0dcf..88b4807305 100644 --- a/mayan/apps/common/compat.py +++ b/mayan/apps/common/compat.py @@ -1,8 +1,14 @@ from __future__ import unicode_literals +import os import types +from django.conf import settings +from django.http.response import StreamingHttpResponse from django.utils import six +from django.utils.six.moves.urllib.parse import quote + +from mayan.apps.mimetype.api import get_mimetype if six.PY3: dict_type = dict @@ -22,3 +28,75 @@ except NameError: FileNotFoundErrorException = IOError else: FileNotFoundErrorException = FileNotFoundError # NOQA + + +class FileResponse(StreamingHttpResponse): + """ + Port of Django's 2.2 FileResponse + Modified to allows downloading non file like content as attachment + A streaming HTTP response class optimized for files. + TODO: To be remove when the code moves to Django 2.2 + """ + block_size = 4096 + + def __init__(self, as_attachment=False, filename='', *args, **kwargs): + self.as_attachment = as_attachment + self.filename = filename + super(FileResponse, self).__init__(*args, **kwargs) + + def _set_as_attachment(self, filename): + if self.as_attachment: + filename = self.filename or os.path.basename(filename) + if filename: + try: + filename.encode('ascii') + file_expr = 'filename="{}"'.format(filename) + except UnicodeEncodeError: + file_expr = "filename*=utf-8''{}".format(quote(filename)) + self['Content-Disposition'] = 'attachment; {}'.format(file_expr) + + def _set_streaming_content(self, value): + if not hasattr(value, 'read'): + self.file_to_stream = None + result = super(FileResponse, self)._set_streaming_content(value) + self._set_as_attachment(filename=self.filename) + + return result + + self.file_to_stream = filelike = value + if hasattr(filelike, 'close'): + self._closable_objects.append(filelike) + value = iter(lambda: filelike.read(self.block_size), b'') + self.set_headers(filelike) + super(FileResponse, self)._set_streaming_content(value) + + def set_headers(self, filelike): + """ + Set some common response headers (Content-Length, Content-Type, and + Content-Disposition) based on the `filelike` response content. + """ + encoding_map = { + 'bzip2': 'application/x-bzip', + 'gzip': 'application/gzip', + 'xz': 'application/x-xz', + } + filename = getattr(filelike, 'name', None) + filename = filename if (isinstance(filename, str) and filename) else self.filename + if os.path.isabs(filename): + self['Content-Length'] = os.path.getsize(filelike.name) + elif hasattr(filelike, 'getbuffer'): + self['Content-Length'] = filelike.getbuffer().nbytes + + if self.get('Content-Type', '').startswith(settings.DEFAULT_CONTENT_TYPE): + if self.file_to_stream: + content_type, encoding = get_mimetype( + file_object=self.file_to_stream, mimetype_only=True + ) + # Encoding isn't set to prevent browsers from automatically + # uncompressing files. + content_type = encoding_map.get(encoding, content_type) + self['Content-Type'] = content_type or 'application/octet-stream' + else: + self['Content-Type'] = 'application/octet-stream' + + self._set_as_attachment(filename=filename) diff --git a/mayan/apps/common/generics.py b/mayan/apps/common/generics.py index 7de15ca51f..129ecd3e3a 100644 --- a/mayan/apps/common/generics.py +++ b/mayan/apps/common/generics.py @@ -11,15 +11,13 @@ from django.utils.translation import ugettext_lazy as _ from django.views.generic import ( FormView as DjangoFormView, DetailView, TemplateView ) +from django.views.generic.base import View from django.views.generic.detail import SingleObjectMixin from django.views.generic.edit import ( CreateView, DeleteView, FormMixin, ModelFormMixin, UpdateView ) from django.views.generic.list import ListView -from django_downloadview import ( - TextIteratorIO, VirtualDownloadView, VirtualFile -) from pure_pagination.mixins import PaginationMixin from mayan.apps.acls.models import AccessControlList @@ -36,11 +34,10 @@ from .literals import ( TEXT_SORT_ORDER_VARIABLE_NAME ) from .mixins import ( - DeleteExtraDataMixin, DynamicFormViewMixin, ExternalObjectMixin, - ExtraContextMixin, FormExtraKwargsMixin, MultipleObjectMixin, - ObjectActionMixin, ObjectNameMixin, - ObjectPermissionCheckMixin, RedirectionMixin, RestrictedQuerysetMixin, - ViewPermissionCheckMixin + DeleteExtraDataMixin, DownloadMixin, DynamicFormViewMixin, + ExternalObjectMixin, ExtraContextMixin, FormExtraKwargsMixin, + MultipleObjectMixin, ObjectActionMixin, ObjectNameMixin, + RedirectionMixin, RestrictedQuerysetMixin, ViewPermissionCheckMixin ) from .settings import setting_paginate_by @@ -491,7 +488,9 @@ class MultipleObjectConfirmActionView( class SimpleView(ViewPermissionCheckMixin, ExtraContextMixin, TemplateView): - pass + """ + Basic template view class with permission check and extra context + """ class SingleObjectCreateView( @@ -657,9 +656,52 @@ class SingleObjectDetailView( return super(SingleObjectDetailView, self).get_queryset() -class SingleObjectDownloadView(ViewPermissionCheckMixin, ObjectPermissionCheckMixin, VirtualDownloadView, SingleObjectMixin): - TextIteratorIO = TextIteratorIO - VirtualFile = VirtualFile +class BaseDownloadView(DownloadMixin, ViewPermissionCheckMixin, View): + def get(self, request, *args, **kwargs): + return self.render_to_response() + + +class SingleObjectDownloadView( + RestrictedQuerysetMixin, SingleObjectMixin, BaseDownloadView +): + def get(self, request, *args, **kwargs): + self.object = self.get_object() + return super(SingleObjectDownloadView, self).get( + request, *args, **kwargs + ) + + def get_download_file_object(self): + return self.object.open() + + def get_download_label(self): + return force_text(self.object) + + +class MultipleObjectDownloadView( + RestrictedQuerysetMixin, MultipleObjectMixin, BaseDownloadView +): + """ + View that support receiving multiple objects via a pk_list query. + """ + def __init__(self, *args, **kwargs): + result = super(MultipleObjectDownloadView, self).__init__(*args, **kwargs) + + if self.__class__.mro()[0].get_queryset != MultipleObjectDownloadView.get_queryset: + raise ImproperlyConfigured( + '%(cls)s is overloading the get_queryset method. Subclasses ' + 'should implement the get_source_queryset method instead. ' % { + 'cls': self.__class__.__name__ + } + ) + + return result + + def get_queryset(self): + try: + return super(MultipleObjectDownloadView, self).get_queryset() + except ImproperlyConfigured: + self.queryset = self.get_source_queryset() + return super(MultipleObjectDownloadView, self).get_queryset() class SingleObjectDynamicFormCreateView( diff --git a/mayan/apps/common/mixins.py b/mayan/apps/common/mixins.py index 861b75df93..b2293d6338 100644 --- a/mayan/apps/common/mixins.py +++ b/mayan/apps/common/mixins.py @@ -13,6 +13,7 @@ from mayan.apps.acls.classes import ModelPermission from mayan.apps.acls.models import AccessControlList from mayan.apps.permissions import Permission +from .compat import FileResponse from .exceptions import ActionError from .forms import DynamicForm from .literals import PK_LIST_SEPARATOR @@ -56,6 +57,29 @@ class DeleteExtraDataMixin(object): return HttpResponseRedirect(redirect_to=success_url) +class DownloadMixin(object): + as_attachment = True + + def get_as_attachment(self): + return self.as_attachment + + def get_download_file_object(self): + raise NotImplementedError( + 'Class must provide a .get_download_file_object() method that ' + 'return a file like object.' + ) + + def get_download_filename(self): + return None + + def render_to_response(self, **response_kwargs): + return FileResponse( + as_attachment=self.get_as_attachment(), + filename=self.get_download_filename(), + streaming_content=self.get_download_file_object() + ) + + class DynamicFormViewMixin(object): form_class = DynamicForm @@ -345,26 +369,6 @@ class ObjectNameMixin(object): return object_name -# TODO: Remove this mixin and replace with restricted queryset -class ObjectPermissionCheckMixin(object): - object_permission = None - - def get_permission_object(self): - return self.get_object() - - def dispatch(self, request, *args, **kwargs): - if self.object_permission: - AccessControlList.objects.check_access( - obj=self.get_permission_object(), - permissions=(self.object_permission,), - user=request.user - ) - - return super( - ObjectPermissionCheckMixin, self - ).dispatch(request, *args, **kwargs) - - class RedirectionMixin(object): action_cancel_redirect = None next_url = None diff --git a/mayan/apps/common/tests/base.py b/mayan/apps/common/tests/base.py index d92d2168ec..052db26e93 100644 --- a/mayan/apps/common/tests/base.py +++ b/mayan/apps/common/tests/base.py @@ -2,8 +2,6 @@ from __future__ import absolute_import, unicode_literals from django.test import TestCase -from django_downloadview import assert_download_response - from mayan.apps.acls.tests.mixins import ACLTestCaseMixin from mayan.apps.converter.tests.mixins import LayerTestCaseMixin from mayan.apps.permissions.tests.mixins import PermissionTestCaseMixin @@ -14,7 +12,7 @@ from mayan.apps.user_management.tests.mixins import UserTestMixin from .mixins import ( ClientMethodsTestCaseMixin, ConnectionsCheckTestCaseMixin, - ContentTypeCheckTestCaseMixin, ModelTestCaseMixin, + ContentTypeCheckTestCaseMixin, DownloadTestCaseMixin, ModelTestCaseMixin, OpenFileCheckTestCaseMixin, RandomPrimaryKeyModelMonkeyPatchMixin, SilenceLoggerTestCaseMixin, TempfileCheckTestCasekMixin, TestViewTestCaseMixin @@ -22,7 +20,8 @@ from .mixins import ( class BaseTestCase( - LayerTestCaseMixin, SilenceLoggerTestCaseMixin, ConnectionsCheckTestCaseMixin, + LayerTestCaseMixin, SilenceLoggerTestCaseMixin, + ConnectionsCheckTestCaseMixin, DownloadTestCaseMixin, RandomPrimaryKeyModelMonkeyPatchMixin, ACLTestCaseMixin, ModelTestCaseMixin, OpenFileCheckTestCaseMixin, PermissionTestCaseMixin, SmartSettingsTestCaseMixin, TempfileCheckTestCasekMixin, UserTestMixin, @@ -31,7 +30,6 @@ class BaseTestCase( """ This is the most basic test case class any test in the project should use. """ - assert_download_response = assert_download_response class GenericViewTestCase( diff --git a/mayan/apps/common/tests/mixins.py b/mayan/apps/common/tests/mixins.py index c6b2eb5527..d0850cb447 100644 --- a/mayan/apps/common/tests/mixins.py +++ b/mayan/apps/common/tests/mixins.py @@ -18,12 +18,16 @@ from django.http import HttpResponse from django.template import Context, Template from django.test.utils import ContextList from django.urls import clear_url_caches, reverse -from django.utils.encoding import force_bytes +from django.utils.encoding import ( + DjangoUnicodeDecodeError, force_bytes, force_text +) from django.utils.six import PY3 from mayan.apps.acls.classes import ModelPermission from mayan.apps.storage.settings import setting_temporary_directory +from ..compat import FileResponse + from .literals import ( TEST_SERVER_HOST, TEST_SERVER_SCHEME, TEST_VIEW_NAME, TEST_VIEW_URL ) @@ -141,6 +145,37 @@ class ContentTypeCheckTestCaseMixin(object): self.client = CustomClient() +class DownloadTestCaseMixin(object): + def assert_download_response( + self, response, content=None, filename=None, is_attachment=None, + mime_type=None + ): + self.assertTrue(isinstance(response, FileResponse)) + + if filename: + self.assertEqual( + response[ + 'Content-Disposition' + ].split('filename="')[1].split('"')[0], filename + ) + + if content: + response_content = b''.join(list(response)) + + try: + response_content = force_text(response_content) + except DjangoUnicodeDecodeError: + """Leave as bytes""" + + self.assertEqual(response_content, content) + + if is_attachment is not None: + self.assertEqual(response['Content-Disposition'], 'attachment') + + if mime_type: + self.assertTrue(response['Content-Type'].startswith(mime_type)) + + class EnvironmentTestCaseMixin(object): def setUp(self): super(EnvironmentTestCaseMixin, self).setUp() diff --git a/mayan/apps/django_gpg/tests/test_views.py b/mayan/apps/django_gpg/tests/test_views.py index bda30a6131..128401f0fd 100644 --- a/mayan/apps/django_gpg/tests/test_views.py +++ b/mayan/apps/django_gpg/tests/test_views.py @@ -1,7 +1,5 @@ from __future__ import absolute_import, unicode_literals -from django_downloadview.test import assert_download_response - from mayan.apps.common.tests.base import GenericViewTestCase from ..models import Key @@ -16,10 +14,10 @@ class KeyViewTestCase(KeyTestMixin, KeyViewTestMixin, GenericViewTestCase): self._create_test_key_private() response = self._request_test_key_download_view() - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 404) def test_key_download_view_with_permission(self): - self.expected_content_types = ('application/octet-stream; charset=utf-8',) + self.expected_content_types = ('text/html; charset=utf-8',) self._create_test_key_private() @@ -28,9 +26,9 @@ class KeyViewTestCase(KeyTestMixin, KeyViewTestMixin, GenericViewTestCase): ) response = self._request_test_key_download_view() - assert_download_response( - self, response=response, content=self.test_key_private.key_data, - basename=self.test_key_private.key_id, + self.assert_download_response( + response=response, content=self.test_key_private.key_data, + filename=self.test_key_private.key_id, ) def test_key_upload_view_no_permission(self): diff --git a/mayan/apps/django_gpg/views.py b/mayan/apps/django_gpg/views.py index ea2c4c29fd..fb6032d72c 100644 --- a/mayan/apps/django_gpg/views.py +++ b/mayan/apps/django_gpg/views.py @@ -3,7 +3,6 @@ from __future__ import absolute_import, unicode_literals import logging from django.contrib import messages -from django.core.files.base import ContentFile from django.template import RequestContext from django.urls import reverse, reverse_lazy from django.utils.translation import ugettext_lazy as _ @@ -56,10 +55,11 @@ class KeyDownloadView(SingleObjectDownloadView): model = Key object_permission = permission_key_download - def get_file(self): - key = self.get_object() + def get_download_file_object(self): + return self.object.key_data - return ContentFile(key.key_data, name=key.key_id) + def get_download_filename(self): + return self.object.key_id class KeyReceive(ConfirmView): diff --git a/mayan/apps/document_parsing/tests/test_views.py b/mayan/apps/document_parsing/tests/test_views.py index 9a62b42530..910e994979 100644 --- a/mayan/apps/document_parsing/tests/test_views.py +++ b/mayan/apps/document_parsing/tests/test_views.py @@ -114,12 +114,10 @@ class DocumentContentViewsTestCase( def test_document_parsing_download_view_no_permission(self): response = self._request_test_document_content_download_view() - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 404) def test_document_parsing_download_view_with_access(self): - self.expected_content_types = ( - 'application/octet-stream; charset=utf-8', - ) + self.expected_content_types = ('text/html; charset=utf-8',) self.grant_access( obj=self.test_document, permission=permission_content_view ) diff --git a/mayan/apps/document_parsing/views.py b/mayan/apps/document_parsing/views.py index 286da29787..47446e6df3 100644 --- a/mayan/apps/document_parsing/views.py +++ b/mayan/apps/document_parsing/views.py @@ -75,13 +75,11 @@ class DocumentContentDownloadView(SingleObjectDownloadView): model = Document object_permission = permission_content_view - def get_file(self): - file_object = DocumentContentDownloadView.TextIteratorIO( - iterator=get_document_content(document=self.get_object()) - ) - return DocumentContentDownloadView.VirtualFile( - file=file_object, name='{}-content'.format(self.get_object()) - ) + def get_download_file_object(self): + return get_document_content(document=self.object) + + def get_download_filename(self): + return '{}-content'.format(self.object) class DocumentPageContentView(SingleObjectDetailView): diff --git a/mayan/apps/document_signatures/tests/test_views.py b/mayan/apps/document_signatures/tests/test_views.py index ff37dd1182..7beabc7130 100644 --- a/mayan/apps/document_signatures/tests/test_views.py +++ b/mayan/apps/document_signatures/tests/test_views.py @@ -1,7 +1,5 @@ from __future__ import absolute_import, unicode_literals -from django_downloadview.test import assert_download_response - from mayan.apps.django_gpg.permissions import permission_key_sign from mayan.apps.django_gpg.tests.mixins import KeyTestMixin from mayan.apps.documents.models import DocumentVersion @@ -287,7 +285,7 @@ class DetachedSignaturesViewTestCase( self._create_test_detached_signature() response = self._request_test_document_version_signature_download_view() - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 404) def test_signature_download_view_with_access(self): self.test_document_path = TEST_SMALL_DOCUMENT_PATH @@ -300,13 +298,13 @@ class DetachedSignaturesViewTestCase( permission=permission_document_version_signature_download ) - self.expected_content_types = ('application/octet-stream; charset=utf-8',) + self.expected_content_types = ('application/octet-stream',) response = self._request_test_document_version_signature_download_view() with self.test_signature.signature_file as file_object: - assert_download_response( - self, response=response, content=file_object.read(), + self.assert_download_response( + response=response, content=file_object.read(), ) def test_signature_upload_view_no_permission(self): diff --git a/mayan/apps/document_signatures/views.py b/mayan/apps/document_signatures/views.py index 6e890104a4..21d1adc1f7 100644 --- a/mayan/apps/document_signatures/views.py +++ b/mayan/apps/document_signatures/views.py @@ -254,12 +254,11 @@ class DocumentVersionSignatureDownloadView(SingleObjectDownloadView): model = DetachedSignature object_permission = permission_document_version_signature_download - def get_file(self): - signature = self.get_object() + def get_download_file_object(self): + return self.object.signature_file - return DocumentVersionSignatureDownloadView.VirtualFile( - signature.signature_file, name=force_text(signature) - ) + def get_download_filename(self): + return force_text(self.object) class DocumentVersionSignatureListView( diff --git a/mayan/apps/documents/api_views.py b/mayan/apps/documents/api_views.py index f8ceb5eee9..6c24dc2823 100644 --- a/mayan/apps/documents/api_views.py +++ b/mayan/apps/documents/api_views.py @@ -6,12 +6,12 @@ from django.http import HttpResponse from django.shortcuts import get_object_or_404 from django.views.decorators.cache import cache_control, patch_cache_control -from django_downloadview import DownloadMixin, VirtualFile from rest_framework import status from rest_framework.response import Response from mayan.apps.acls.models import AccessControlList from mayan.apps.rest_api import generics +from mayan.apps.common.generics import DownloadMixin from .literals import DOCUMENT_IMAGE_TASK_TIMEOUT from .models import ( @@ -113,15 +113,11 @@ class APIDocumentDownloadView(DownloadMixin, generics.RetrieveAPIView): } queryset = Document.objects.all() - def get_encoding(self): - return self.get_object().latest_version.encoding + def get_download_file_object(self): + return self.get_object().open() - 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_download_filename(self): + return self.get_object().label def get_serializer(self, *args, **kwargs): return None @@ -349,10 +345,11 @@ class APIDocumentVersionDownloadView(DownloadMixin, generics.RetrieveAPIView): ) return document - def get_encoding(self): - return self.get_object().encoding + def get_download_file_object(self): + instance = self.get_object() + return instance.open() - def get_file(self): + def get_download_filename(self): preserve_extension = self.request.GET.get( 'preserve_extension', self.request.POST.get( 'preserve_extension', False @@ -362,15 +359,10 @@ class APIDocumentVersionDownloadView(DownloadMixin, generics.RetrieveAPIView): preserve_extension = preserve_extension == 'true' or preserve_extension == 'True' instance = self.get_object() - return VirtualFile( - instance.file, name=instance.get_rendered_string( - preserve_extension=preserve_extension - ) + return instance.get_rendered_string( + preserve_extension=preserve_extension ) - def get_mimetype(self): - return self.get_object().mimetype - def get_serializer(self, *args, **kwargs): return None diff --git a/mayan/apps/documents/forms/document_forms.py b/mayan/apps/documents/forms/document_forms.py index c1febeffbe..f26a3b8d32 100644 --- a/mayan/apps/documents/forms/document_forms.py +++ b/mayan/apps/documents/forms/document_forms.py @@ -45,7 +45,9 @@ class DocumentDownloadForm(forms.Form): super(DocumentDownloadForm, self).__init__(*args, **kwargs) if self.queryset.count() > 1: self.fields['compressed'].initial = True - self.fields['compressed'].widget.attrs.update({'disabled': True}) + self.fields['compressed'].widget.attrs.update( + {'disabled': 'disabled'} + ) class DocumentForm(forms.ModelForm): diff --git a/mayan/apps/documents/tests/mixins.py b/mayan/apps/documents/tests/mixins.py index 7c314aaf17..cafa849a7a 100644 --- a/mayan/apps/documents/tests/mixins.py +++ b/mayan/apps/documents/tests/mixins.py @@ -204,13 +204,20 @@ class DocumentViewTestMixin(object): } ) - def _request_document_download_form_view(self): + def _request_document_download_form_get_view(self): return self.get( viewname='documents:document_download_form', kwargs={ 'pk': self.test_document.pk } ) + def _request_document_download_form_post_view(self): + return self.post( + viewname='documents:document_download_form', kwargs={ + 'pk': self.test_document.pk + } + ) + def _request_document_download_view(self): return self.get( viewname='documents:document_download', kwargs={ diff --git a/mayan/apps/documents/tests/test_api.py b/mayan/apps/documents/tests/test_api.py index a7fca309af..8ebc76da72 100644 --- a/mayan/apps/documents/tests/test_api.py +++ b/mayan/apps/documents/tests/test_api.py @@ -4,7 +4,6 @@ import time from django.utils.encoding import force_text -from django_downloadview import assert_download_response from rest_framework import status from mayan.apps.rest_api.tests.base import BaseAPITestCase @@ -213,12 +212,10 @@ class DocumentAPIViewTestCase( self.assertEqual(response.status_code, status.HTTP_200_OK) with self.test_document.open() as file_object: - assert_download_response( - self, response, content=file_object.read(), - basename=TEST_SMALL_DOCUMENT_FILENAME, - mime_type='{}; charset=utf-8'.format( - self.test_document.file_mimetype - ) + self.assert_download_response( + response=response, content=file_object.read(), + filename=TEST_SMALL_DOCUMENT_FILENAME, + mime_type=self.test_document.file_mimetype ) def test_document_api_upload_view_no_permission(self): @@ -418,12 +415,10 @@ class DocumentVersionAPIViewTestCase( self.assertEqual(response.status_code, status.HTTP_200_OK) with self.test_document.latest_version.open() as file_object: - assert_download_response( - self, response, content=file_object.read(), - basename=force_text(self.test_document.latest_version), - mime_type='{}; charset=utf-8'.format( - self.test_document.file_mimetype - ) + self.assert_download_response( + response=response, content=file_object.read(), + filename=force_text(self.test_document.latest_version), + mime_type=self.test_document.file_mimetype ) def test_document_version_api_download_preserve_extension_view(self): @@ -440,13 +435,11 @@ class DocumentVersionAPIViewTestCase( ) with self.test_document.latest_version.open() as file_object: - assert_download_response( - self, response, content=file_object.read(), - basename=self.test_document.latest_version.get_rendered_string( + self.assert_download_response( + response=response, content=file_object.read(), + filename=self.test_document.latest_version.get_rendered_string( preserve_extension=True - ), mime_type='{}; charset=utf-8'.format( - self.test_document.file_mimetype - ) + ), mime_type=self.test_document.file_mimetype ) def test_document_version_api_list_view_no_permission(self): diff --git a/mayan/apps/documents/tests/test_document_views.py b/mayan/apps/documents/tests/test_document_views.py index 4a0a3fed7a..c5dc1f1a91 100644 --- a/mayan/apps/documents/tests/test_document_views.py +++ b/mayan/apps/documents/tests/test_document_views.py @@ -166,33 +166,40 @@ class DocumentViewTestCase( Document.objects.first().document_type, document_type_2 ) - def test_document_download_form_view_no_permission(self): - response = self._request_document_download_form_view() - self.assertNotContains( - response=response, text=self.test_document.label, status_code=200 - ) + def test_document_download_form_get_view_no_permission(self): + response = self._request_document_download_form_get_view() + self.assertEqual(response.status_code, 404) - def test_document_download_form_view_with_access(self): + def test_document_download_form_get_view_with_access(self): self.grant_access( obj=self.test_document, permission=permission_document_download ) - response = self._request_document_download_form_view() + response = self._request_document_download_form_get_view() self.assertContains( response=response, text=self.test_document.label, status_code=200 ) + def test_document_download_form_post_view_no_permission(self): + response = self._request_document_download_form_post_view() + self.assertEqual(response.status_code, 404) + + def test_document_download_form_post_view_with_access(self): + self.grant_access( + obj=self.test_document, permission=permission_document_download + ) + response = self._request_document_download_form_post_view() + self.assertEqual(response.status_code, 302) + def test_document_download_view_no_permission(self): response = self._request_document_download_view() - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 404) def test_document_download_view_with_permission(self): # Set the expected_content_types for # common.tests.mixins.ContentTypeCheckMixin self.expected_content_types = ( - '{}; charset=utf-8'.format( - self.test_document.file_mimetype - ), + self.test_document.file_mimetype, ) self.grant_access( @@ -205,21 +212,19 @@ class DocumentViewTestCase( with self.test_document.open() as file_object: self.assert_download_response( response=response, content=file_object.read(), - basename=TEST_SMALL_DOCUMENT_FILENAME, + filename=TEST_SMALL_DOCUMENT_FILENAME, mime_type=self.test_document.file_mimetype ) def test_document_multiple_download_view_no_permission(self): response = self._request_document_multiple_download_view() - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 404) def test_document_multiple_download_view_with_permission(self): # Set the expected_content_types for # common.tests.mixins.ContentTypeCheckMixin self.expected_content_types = ( - '{}; charset=utf-8'.format( - self.test_document.file_mimetype - ), + self.test_document.file_mimetype, ) self.grant_access( obj=self.test_document, permission=permission_document_download @@ -231,21 +236,19 @@ class DocumentViewTestCase( with self.test_document.open() as file_object: self.assert_download_response( response=response, content=file_object.read(), - basename=TEST_SMALL_DOCUMENT_FILENAME, + filename=TEST_SMALL_DOCUMENT_FILENAME, mime_type=self.test_document.file_mimetype ) def test_document_version_download_view_no_permission(self): response = self._request_document_version_download() - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 404) def test_document_version_download_view_with_permission(self): # Set the expected_content_types for # common.tests.mixins.ContentTypeCheckMixin self.expected_content_types = ( - '{}; charset=utf-8'.format( - self.test_document.latest_version.mimetype - ), + self.test_document.latest_version.mimetype, ) self.grant_access( @@ -258,19 +261,15 @@ class DocumentViewTestCase( with self.test_document.open() as file_object: self.assert_download_response( response=response, content=file_object.read(), - basename=force_text(self.test_document.latest_version), - mime_type='{}; charset=utf-8'.format( - self.test_document.latest_version.mimetype - ) + filename=force_text(self.test_document.latest_version), + mime_type=self.test_document.latest_version.mimetype ) def test_document_version_download_preserve_extension_view_with_permission(self): # Set the expected_content_types for # common.tests.mixins.ContentTypeCheckMixin self.expected_content_types = ( - '{}; charset=utf-8'.format( - self.test_document.latest_version.mimetype - ), + self.test_document.latest_version.mimetype, ) self.grant_access( @@ -285,11 +284,9 @@ class DocumentViewTestCase( with self.test_document.open() as file_object: self.assert_download_response( response=response, content=file_object.read(), - basename=self.test_document.latest_version.get_rendered_string( + filename=self.test_document.latest_version.get_rendered_string( preserve_extension=True - ), mime_type='{}; charset=utf-8'.format( - self.test_document.latest_version.mimetype - ) + ), mime_type=self.test_document.latest_version.mimetype ) def test_document_update_page_count_view_no_permission(self): diff --git a/mayan/apps/documents/tests/test_events.py b/mayan/apps/documents/tests/test_events.py index f2e87dd56a..6178ed7fef 100644 --- a/mayan/apps/documents/tests/test_events.py +++ b/mayan/apps/documents/tests/test_events.py @@ -1,7 +1,6 @@ from __future__ import unicode_literals from actstream.models import Action -from django_downloadview import assert_download_response from ..events import ( event_document_download, event_document_trashed, event_document_view @@ -45,11 +44,11 @@ class DocumentEventsTestCase( def test_document_download_event_no_permission(self): response = self._request_test_document_download_view() - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 404) self.assertEqual(list(Action.objects.any(obj=self.test_document)), []) def test_document_download_event_with_access(self): - self.expected_content_types = ('image/png; charset=utf-8',) + self.expected_content_types = ('image/png',) self.grant_access( obj=self.test_document, permission=permission_document_download @@ -59,8 +58,8 @@ class DocumentEventsTestCase( # Download the file to close the file descriptor with self.test_document.open() as file_object: - assert_download_response( - self, response, content=file_object.read(), + self.assert_download_response( + response=response, content=file_object.read(), mime_type=self.test_document.file_mimetype ) diff --git a/mayan/apps/documents/urls.py b/mayan/apps/documents/urls.py index 6be733e983..722d6eb81b 100644 --- a/mayan/apps/documents/urls.py +++ b/mayan/apps/documents/urls.py @@ -272,6 +272,11 @@ urlpatterns_document_versions = [ view=DocumentVersionDownloadView.as_view(), name='document_version_download' ), + url( + regex=r'^documents/versions/multiple/download/$', + view=DocumentVersionDownloadView.as_view(), + name='document_multiple_version_download' + ), url( regex=r'^documents/versions/(?P\d+)/revert/$', view=DocumentVersionRevertView.as_view(), diff --git a/mayan/apps/documents/views/document_version_views.py b/mayan/apps/documents/views/document_version_views.py index 4c53bbff36..8fba8f9c03 100644 --- a/mayan/apps/documents/views/document_version_views.py +++ b/mayan/apps/documents/views/document_version_views.py @@ -14,8 +14,7 @@ from ..events import event_document_view from ..forms import DocumentVersionDownloadForm, DocumentVersionPreviewForm from ..models import Document, DocumentVersion from ..permissions import ( - permission_document_download, permission_document_version_revert, - permission_document_version_view + permission_document_version_revert, permission_document_version_view ) from .document_views import DocumentDownloadFormView, DocumentDownloadView @@ -31,11 +30,11 @@ logger = logging.getLogger(__name__) class DocumentVersionDownloadFormView(DocumentDownloadFormView): form_class = DocumentVersionDownloadForm model = DocumentVersion - multiple_download_view = None + pk_url_kwarg = 'pk' querystring_form_fields = ( 'compressed', 'zip_filename', 'preserve_extension' ) - single_download_view = 'documents:document_version_download' + viewname = 'documents:document_multiple_version_download' def get_extra_context(self): result = super( @@ -48,31 +47,12 @@ class DocumentVersionDownloadFormView(DocumentDownloadFormView): return result - def get_document_queryset(self): - id_list = self.request.GET.get( - 'id_list', self.request.POST.get('id_list', '') - ) - - if not id_list: - id_list = self.kwargs['pk'] - - return self.model.objects.filter( - pk__in=id_list.split(',') - ) - class DocumentVersionDownloadView(DocumentDownloadView): model = DocumentVersion - object_permission = permission_document_download + pk_url_kwarg = 'pk' - @staticmethod - def get_item_file(item): - return item.file - - def get_encoding(self): - return self.get_object().encoding - - def get_item_label(self, item): + def get_item_filename(self, item): preserve_extension = self.request.GET.get( 'preserve_extension', self.request.POST.get( 'preserve_extension', False @@ -83,9 +63,6 @@ class DocumentVersionDownloadView(DocumentDownloadView): return item.get_rendered_string(preserve_extension=preserve_extension) - def get_mimetype(self): - return self.get_object().mimetype - class DocumentVersionListView(ExternalObjectMixin, SingleObjectListView): external_object_class = Document diff --git a/mayan/apps/documents/views/document_views.py b/mayan/apps/documents/views/document_views.py index 4e69f6edf9..f56be5a86e 100644 --- a/mayan/apps/documents/views/document_views.py +++ b/mayan/apps/documents/views/document_views.py @@ -2,22 +2,23 @@ from __future__ import absolute_import, unicode_literals import logging +from furl import furl + from django.conf import settings from django.contrib import messages -from django.core.exceptions import PermissionDenied from django.db import transaction from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404 from django.urls import reverse -from django.utils.http import urlencode +from django.utils.encoding import force_text from django.utils.translation import ugettext_lazy as _, ungettext from mayan.apps.acls.models import AccessControlList from mayan.apps.common.compressed_files import ZipArchive from mayan.apps.common.generics import ( - FormView, MultipleObjectConfirmActionView, MultipleObjectFormActionView, - SingleObjectDetailView, SingleObjectDownloadView, SingleObjectEditView, - SingleObjectListView + FormView, MultipleObjectConfirmActionView, MultipleObjectDownloadView, + MultipleObjectFormActionView, SingleObjectDetailView, + SingleObjectEditView, SingleObjectListView ) from mayan.apps.converter.layers import layer_saved_transformations from mayan.apps.converter.permissions import ( @@ -154,55 +155,36 @@ class DocumentDocumentTypeEditView(MultipleObjectFormActionView): ) -class DocumentDownloadFormView(FormView): +class DocumentDownloadFormView(MultipleObjectFormActionView): form_class = DocumentDownloadForm model = Document - multiple_download_view = 'documents:document_multiple_download' + object_permission = permission_document_download + pk_url_kwarg = 'pk' querystring_form_fields = ('compressed', 'zip_filename') - single_download_view = 'documents:document_download' + viewname = 'documents:document_multiple_download' def form_valid(self, form): - querystring_dictionary = {} + # Turn a queryset into a comma separated list of primary keys + id_list = ','.join( + [ + force_text(pk) for pk in self.get_object_list().values_list('pk', flat=True) + ] + ) + # Construct URL with querystring to pass on to the next view + url = furl( + args={ + 'id_list': id_list + }, path=reverse(viewname=self.viewname) + ) + + # Pass the form field data as URL querystring to the next view for field in self.querystring_form_fields: data = form.cleaned_data[field] if data: - querystring_dictionary[field] = data + url.args[field] = data - querystring_dictionary.update( - { - 'id_list': ','.join( - map(str, self.queryset.values_list('pk', flat=True)) - ) - } - ) - - querystring = urlencode(querystring_dictionary, doseq=True) - - if self.queryset.count() > 1: - url = reverse(self.multiple_download_view) - else: - url = reverse( - viewname=self.single_download_view, kwargs={ - 'pk': self.queryset.first().pk - } - ) - - return HttpResponseRedirect( - redirect_to='{}?{}'.format(url, querystring) - ) - - def get_document_queryset(self): - id_list = self.request.GET.get( - 'id_list', self.request.POST.get('id_list', '') - ) - - if not id_list: - id_list = self.kwargs['pk'] - - return self.model.objects.filter( - pk__in=id_list.split(',') - ) + return HttpResponseRedirect(redirect_to=url.tostr()) def get_extra_context(self): subtemplates_list = [ @@ -210,7 +192,6 @@ class DocumentDownloadFormView(FormView): 'name': 'appearance/generic_list_items_subtemplate.html', 'context': { 'object_list': self.queryset, - 'hide_link': True, 'hide_links': True, 'hide_multi_item_actions': True, } @@ -230,21 +211,14 @@ class DocumentDownloadFormView(FormView): def get_form_kwargs(self): kwargs = super(DocumentDownloadFormView, self).get_form_kwargs() - self.queryset = self.get_queryset() + self.queryset = self.get_object_list() kwargs.update({'queryset': self.queryset}) return kwargs - def get_queryset(self): - return AccessControlList.objects.restrict_queryset( - permission=permission_document_download, - queryset=self.get_document_queryset(), user=self.request.user - ) - -class DocumentDownloadView(SingleObjectDownloadView): +class DocumentDownloadView(MultipleObjectDownloadView): model = Document - # Set to None to disable the .get_object call - object_permission = None + object_permission = permission_document_download @staticmethod def commit_event(item, request): @@ -259,39 +233,23 @@ class DocumentDownloadView(SingleObjectDownloadView): target=item.document ) - @staticmethod - def get_item_file(item): - return item.open() - - def get_document_queryset(self): - id_list = self.request.GET.get( - 'id_list', self.request.POST.get('id_list', '') - ) - - if not id_list: - id_list = self.kwargs['pk'] - - queryset = self.model.objects.filter(pk__in=id_list.split(',')) - - return AccessControlList.objects.restrict_queryset( - permission=permission_document_download, queryset=queryset, - user=self.request.user - ) - - def get_file(self): - queryset = self.get_document_queryset() - zip_filename = self.request.GET.get( + def get_archive_filename(self): + return self.request.GET.get( 'zip_filename', DEFAULT_ZIP_FILENAME ) + def get_download_file_object(self): + queryset = self.get_object_list() + zip_filename = self.get_archive_filename() + if self.request.GET.get('compressed') == 'True' or queryset.count() > 1: compressed_file = ZipArchive() compressed_file.create() for item in queryset: - with DocumentDownloadView.get_item_file(item=item) as file_object: + with item.open() as file_object: compressed_file.add_file( file_object=file_object, - filename=self.get_item_label(item=item) + filename=self.get_item_filename(item=item) ) DocumentDownloadView.commit_event( item=item, request=self.request @@ -299,24 +257,22 @@ class DocumentDownloadView(SingleObjectDownloadView): compressed_file.close() - return DocumentDownloadView.VirtualFile( - compressed_file.as_file(zip_filename), name=zip_filename - ) + return compressed_file.as_file(zip_filename) else: item = queryset.first() - if item: - DocumentDownloadView.commit_event( - item=item, request=self.request - ) - else: - raise PermissionDenied - - return DocumentDownloadView.VirtualFile( - DocumentDownloadView.get_item_file(item=item), - name=self.get_item_label(item=item) + DocumentDownloadView.commit_event( + item=item, request=self.request ) + return item.open() - def get_item_label(self, item): + def get_download_filename(self): + queryset = self.get_object_list() + if self.request.GET.get('compressed') == 'True' or queryset.count() > 1: + return self.get_archive_filename() + else: + return self.get_item_filename(item=queryset.first()) + + def get_item_filename(self, item): return item.label diff --git a/mayan/apps/ocr/tests/test_views.py b/mayan/apps/ocr/tests/test_views.py index 8e83b43bb6..f60561ee7d 100644 --- a/mayan/apps/ocr/tests/test_views.py +++ b/mayan/apps/ocr/tests/test_views.py @@ -169,11 +169,11 @@ class OCRViewsTestCase(OCRViewTestMixin, GenericDocumentViewTestCase): self.test_document.submit_for_ocr() response = self._request_document_ocr_download_view() - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 404) def test_document_ocr_download_view_with_access(self): self.test_document.submit_for_ocr() - self.expected_content_types = ('application/octet-stream; charset=utf-8',) + self.expected_content_types = ('text/html; charset=utf-8',) self.grant_access( obj=self.test_document, permission=permission_ocr_content_view diff --git a/mayan/apps/ocr/views.py b/mayan/apps/ocr/views.py index 41b978388e..d58b77b6c8 100644 --- a/mayan/apps/ocr/views.py +++ b/mayan/apps/ocr/views.py @@ -211,10 +211,8 @@ class DocumentOCRDownloadView(SingleObjectDownloadView): model = Document object_permission = permission_ocr_content_view - def get_file(self): - file_object = DocumentOCRDownloadView.TextIteratorIO( - iterator=get_document_ocr_content(document=self.get_object()) - ) - return DocumentOCRDownloadView.VirtualFile( - file=file_object, name='{}-OCR'.format(self.get_object()) - ) + def get_download_file_object(self): + return get_document_ocr_content(document=self.object) + + def get_download_filename(self): + return '{}-OCR'.format(self.object) diff --git a/removals.txt b/removals.txt index bf088e6961..e9b83bc02d 100644 --- a/removals.txt +++ b/removals.txt @@ -2,6 +2,7 @@ cssmin django-autoadmin django-celery +django-downloadview django-environ django-suit django-compressor