Refactor and implement download code natively

- Use modified port of Django 2.2 FileResponse.
- Remove Django DownloadView library.

Signed-off-by: Roberto Rosario <roberto.rosario@mayan-edms.com>

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

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)

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,
+    DeleteExtraDataMixin, DownloadMixin, DynamicFormViewMixin,
-    ExtraContextMixin, FormExtraKwargsMixin, MultipleObjectMixin,
+    ExternalObjectMixin, ExtraContextMixin, FormExtraKwargsMixin,
-    ObjectActionMixin, ObjectNameMixin,
+    MultipleObjectMixin, ObjectActionMixin, ObjectNameMixin,
-    ObjectPermissionCheckMixin, RedirectionMixin, RestrictedQuerysetMixin,
+    RedirectionMixin, RestrictedQuerysetMixin, ViewPermissionCheckMixin
-    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):
+class BaseDownloadView(DownloadMixin, ViewPermissionCheckMixin, View):
-    TextIteratorIO = TextIteratorIO
+    def get(self, request, *args, **kwargs):
-    VirtualFile = VirtualFile
+        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(

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

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(

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

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.assert_download_response(
-            self, response=response, content=self.test_key_private.key_data,
+            response=response, content=self.test_key_private.key_data,
-            basename=self.test_key_private.key_id,
+            filename=self.test_key_private.key_id,
         )
 
     def test_key_upload_view_no_permission(self):

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):
+    def get_download_file_object(self):
-        key = self.get_object()
+        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):

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 = (
+        self.expected_content_types = ('text/html; charset=utf-8',)
-            'application/octet-stream; charset=utf-8',
-        )
         self.grant_access(
             obj=self.test_document, permission=permission_content_view
         )

mayan/apps/document_parsing/views.py

@@ -75,13 +75,11 @@ class DocumentContentDownloadView(SingleObjectDownloadView):
     model = Document
     object_permission = permission_content_view
 
-    def get_file(self):
+    def get_download_file_object(self):
-        file_object = DocumentContentDownloadView.TextIteratorIO(
+        return get_document_content(document=self.object)
-            iterator=get_document_content(document=self.get_object())
+
-        )
+    def get_download_filename(self):
-        return DocumentContentDownloadView.VirtualFile(
+        return '{}-content'.format(self.object)
-            file=file_object, name='{}-content'.format(self.get_object())
-        )
 
 
 class DocumentPageContentView(SingleObjectDetailView):

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.assert_download_response(
-                self, response=response, content=file_object.read(),
+                response=response, content=file_object.read(),
             )
 
     def test_signature_upload_view_no_permission(self):

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):
+    def get_download_file_object(self):
-        signature = self.get_object()
+        return self.object.signature_file
 
-        return DocumentVersionSignatureDownloadView.VirtualFile(
+    def get_download_filename(self):
-            signature.signature_file, name=force_text(signature)
+        return force_text(self.object)
-        )
 
 
 class DocumentVersionSignatureListView(

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):
+    def get_download_file_object(self):
-        return self.get_object().latest_version.encoding
+        return self.get_object().open()
 
-    def get_file(self):
+    def get_download_filename(self):
-        instance = self.get_object()
+        return self.get_object().label
-        return VirtualFile(instance.latest_version.file, name=instance.label)
-
-    def get_mimetype(self):
-        return self.get_object().latest_version.mimetype
 
     def get_serializer(self, *args, **kwargs):
         return None
@@ -349,10 +345,11 @@ class APIDocumentVersionDownloadView(DownloadMixin, generics.RetrieveAPIView):
         )
         return document
 
-    def get_encoding(self):
+    def get_download_file_object(self):
-        return self.get_object().encoding
+        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(
+        return instance.get_rendered_string(
-            instance.file, name=instance.get_rendered_string(
+            preserve_extension=preserve_extension
-                preserve_extension=preserve_extension
-            )
         )
 
-    def get_mimetype(self):
-        return self.get_object().mimetype
-
     def get_serializer(self, *args, **kwargs):
         return None
 

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

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={

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.assert_download_response(
-                self, response, content=file_object.read(),
+                response=response, content=file_object.read(),
-                basename=TEST_SMALL_DOCUMENT_FILENAME,
+                filename=TEST_SMALL_DOCUMENT_FILENAME,
-                mime_type='{}; charset=utf-8'.format(
+                mime_type=self.test_document.file_mimetype
-                    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.assert_download_response(
-                self, response, content=file_object.read(),
+                response=response, content=file_object.read(),
-                basename=force_text(self.test_document.latest_version),
+                filename=force_text(self.test_document.latest_version),
-                mime_type='{}; charset=utf-8'.format(
+                mime_type=self.test_document.file_mimetype
-                    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.assert_download_response(
-                self, response, content=file_object.read(),
+                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(
+                ), mime_type=self.test_document.file_mimetype
-                    self.test_document.file_mimetype
-                )
             )
 
     def test_document_version_api_list_view_no_permission(self):

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):
+    def test_document_download_form_get_view_no_permission(self):
-        response = self._request_document_download_form_view()
+        response = self._request_document_download_form_get_view()
-        self.assertNotContains(
+        self.assertEqual(response.status_code, 404)
-            response=response, text=self.test_document.label, status_code=200
-        )
 
-    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),
+                filename=force_text(self.test_document.latest_version),
-                mime_type='{}; charset=utf-8'.format(
+                mime_type=self.test_document.latest_version.mimetype
-                    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(
+                ), mime_type=self.test_document.latest_version.mimetype
-                    self.test_document.latest_version.mimetype
-                )
             )
 
     def test_document_update_page_count_view_no_permission(self):

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.assert_download_response(
-                self, response, content=file_object.read(),
+                response=response, content=file_object.read(),
                 mime_type=self.test_document.file_mimetype
             )
 

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<pk>\d+)/revert/$',
         view=DocumentVersionRevertView.as_view(),

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_revert, permission_document_version_view
-    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_filename(self, item):
-    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(
                 '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

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,
+    FormView, MultipleObjectConfirmActionView, MultipleObjectDownloadView,
-    SingleObjectDetailView, SingleObjectDownloadView, SingleObjectEditView,
+    MultipleObjectFormActionView, SingleObjectDetailView,
-    SingleObjectListView
+    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(
+        return HttpResponseRedirect(redirect_to=url.tostr())
-            {
-                '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(',')
-        )
 
     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(MultipleObjectDownloadView):
-class DocumentDownloadView(SingleObjectDownloadView):
     model = Document
-    # Set to None to disable the .get_object call
+    object_permission = permission_document_download
-    object_permission = None
 
     @staticmethod
     def commit_event(item, request):
@@ -259,39 +233,23 @@ class DocumentDownloadView(SingleObjectDownloadView):
                 target=item.document
             )
 
-    @staticmethod
+    def get_archive_filename(self):
-    def get_item_file(item):
+        return self.request.GET.get(
-        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(
             '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(
+            return compressed_file.as_file(zip_filename)
-                compressed_file.as_file(zip_filename), name=zip_filename
-            )
         else:
             item = queryset.first()
-            if item:
+            DocumentDownloadView.commit_event(
-                DocumentDownloadView.commit_event(
+                item=item, request=self.request
-                    item=item, request=self.request
-                )
-            else:
-                raise PermissionDenied
-
-            return DocumentDownloadView.VirtualFile(
-                DocumentDownloadView.get_item_file(item=item),
-                name=self.get_item_label(item=item)
             )
+            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
 
 

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

mayan/apps/ocr/views.py

@@ -211,10 +211,8 @@ class DocumentOCRDownloadView(SingleObjectDownloadView):
     model = Document
     object_permission = permission_ocr_content_view
 
-    def get_file(self):
+    def get_download_file_object(self):
-        file_object = DocumentOCRDownloadView.TextIteratorIO(
+        return get_document_ocr_content(document=self.object)
-            iterator=get_document_ocr_content(document=self.get_object())
+
-        )
+    def get_download_filename(self):
-        return DocumentOCRDownloadView.VirtualFile(
+        return '{}-OCR'.format(self.object)
-            file=file_object, name='{}-OCR'.format(self.get_object())
-        )

removals.txt

@@ -2,6 +2,7 @@
 cssmin
 django-autoadmin
 django-celery
+django-downloadview
 django-environ
 django-suit
 django-compressor