diff --git a/django_downloadview/exceptions.py b/django_downloadview/exceptions.py new file mode 100644 index 0000000..064dc45 --- /dev/null +++ b/django_downloadview/exceptions.py @@ -0,0 +1,11 @@ +# -*- coding: utf-8 -*- +"""Custom exceptions.""" + + +class FileNotFound(IOError): + """Requested file does not exist. + + This exception is to be raised when operations (such as read) fail because + file does not exist (whatever the storage or location). + + """ diff --git a/django_downloadview/tests/views.py b/django_downloadview/tests/views.py index c539d16..8beab15 100644 --- a/django_downloadview/tests/views.py +++ b/django_downloadview/tests/views.py @@ -6,11 +6,13 @@ try: except ImportError: import mock +from django.http import Http404 from django.http.response import HttpResponseNotModified import django.test +from django_downloadview import exceptions from django_downloadview.test import setup_view -from django_downloadview.views import base +from django_downloadview import views class DownloadMixinTestCase(unittest.TestCase): @@ -21,13 +23,13 @@ class DownloadMixinTestCase(unittest.TestCase): Subclasses must implement it! """ - mixin = base.DownloadMixin() + mixin = views.DownloadMixin() with self.assertRaises(NotImplementedError): mixin.get_file() def test_get_basename(self): """DownloadMixin.get_basename() returns basename attribute.""" - mixin = base.DownloadMixin() + mixin = views.DownloadMixin() self.assertEqual(mixin.get_basename(), None) mixin.basename = 'fake' self.assertEqual(mixin.get_basename(), 'fake') @@ -42,7 +44,7 @@ class DownloadMixinTestCase(unittest.TestCase): file_wrapper = mock.Mock() file_wrapper.was_modified_since = mock.Mock( return_value=mock.sentinel.was_modified) - mixin = base.DownloadMixin() + mixin = views.DownloadMixin() self.assertIs( mixin.was_modified_since(file_wrapper, mock.sentinel.since), mock.sentinel.was_modified) @@ -66,7 +68,7 @@ class DownloadMixinTestCase(unittest.TestCase): file_wrapper.modified_time = mock.sentinel.modified_time was_modified_since_mock = mock.Mock( return_value=mock.sentinel.was_modified) - mixin = base.DownloadMixin() + mixin = views.DownloadMixin() with mock.patch('django_downloadview.views.base.was_modified_since', new=was_modified_since_mock): self.assertIs( @@ -97,20 +99,20 @@ class DownloadMixinTestCase(unittest.TestCase): side_effect=NotImplementedError) type(file_wrapper).modified_time = mock.PropertyMock( side_effect=NotImplementedError) - mixin = base.DownloadMixin() + mixin = views.DownloadMixin() self.assertIs( mixin.was_modified_since(file_wrapper, 'fake since'), True) def test_not_modified_response(self): "DownloadMixin.not_modified_response returns HttpResponseNotModified." - mixin = base.DownloadMixin() + mixin = views.DownloadMixin() response = mixin.not_modified_response() self.assertTrue(isinstance(response, HttpResponseNotModified)) def test_download_response(self): "DownloadMixin.download_response() returns download response instance." - mixin = base.DownloadMixin() + mixin = views.DownloadMixin() mixin.file_instance = mock.sentinel.file_wrapper response_factory = mock.Mock(return_value=mock.sentinel.response) mixin.response_class = response_factory @@ -126,7 +128,7 @@ class DownloadMixinTestCase(unittest.TestCase): """DownloadMixin.render_to_response() respects HTTP_IF_MODIFIED_SINCE header (calls ``not_modified_response()``).""" # Setup. - mixin = base.DownloadMixin() + mixin = views.DownloadMixin() mixin.request = django.test.RequestFactory().get( '/dummy-url', HTTP_IF_MODIFIED_SINCE=mock.sentinel.http_if_modified_since) @@ -147,7 +149,7 @@ class DownloadMixinTestCase(unittest.TestCase): def test_render_to_response_modified(self): """DownloadMixin.render_to_response() calls download_response().""" # Setup. - mixin = base.DownloadMixin() + mixin = views.DownloadMixin() mixin.request = django.test.RequestFactory().get( '/dummy-url', HTTP_IF_MODIFIED_SINCE=None) @@ -163,6 +165,24 @@ class DownloadMixinTestCase(unittest.TestCase): self.assertEqual(mixin.was_modified_since.call_count, 0) mixin.download_response.assert_called_once_with() + def test_render_to_response_file_not_found(self): + "DownloadMixin.render_to_response() calls file_not_found_response()." + # Setup. + mixin = views.DownloadMixin() + mixin.request = django.test.RequestFactory().get('/dummy-url') + mixin.get_file = mock.Mock(side_effect=exceptions.FileNotFound) + mixin.file_not_found_response = mock.Mock() + # Run. + mixin.render_to_response() + # Check. + mixin.file_not_found_response.assert_called_once_with() + + def test_file_not_found_response(self): + """DownloadMixin.file_not_found_response() raises Http404.""" + mixin = views.DownloadMixin() + with self.assertRaises(Http404): + mixin.file_not_found_response() + class BaseDownloadViewTestCase(unittest.TestCase): "Tests around :class:`django_downloadviews.views.base.BaseDownloadView`." @@ -171,9 +191,37 @@ class BaseDownloadViewTestCase(unittest.TestCase): request = django.test.RequestFactory().get('/dummy-url') args = ['dummy-arg'] kwargs = {'dummy': 'kwarg'} - view = setup_view(base.BaseDownloadView(), request, *args, **kwargs) + view = setup_view(views.BaseDownloadView(), request, *args, **kwargs) view.render_to_response = mock.Mock( return_value=mock.sentinel.response) response = view.get(request, *args, **kwargs) self.assertIs(response, mock.sentinel.response) view.render_to_response.assert_called_once_with() + + +class ObjectDownloadViewTestCase(unittest.TestCase): + "Tests for :class:`django_downloadviews.views.object.ObjectDownloadView`." + def test_get_file_ok(self): + "ObjectDownloadView.get_file() returns ``file`` field by default." + view = setup_view(views.ObjectDownloadView(), 'fake request') + view.object = mock.Mock(spec=['file']) + view.get_file() + + def test_get_file_wrong_field(self): + """ObjectDownloadView.get_file() raises FileNotFound if field does not + exist.""" + view = setup_view(views.ObjectDownloadView(file_field='other_field'), + 'fake request') + view.object = mock.Mock(spec=['file']) + with self.assertRaises(exceptions.FileNotFound): + view.get_file() + + def test_get_file_empty_field(self): + """ObjectDownloadView.get_file() raises FileNotFound if field does not + exist.""" + view = setup_view(views.ObjectDownloadView(file_field='other_field'), + 'fake request') + view.object = mock.Mock() + view.object.other_field = None + with self.assertRaises(exceptions.FileNotFound): + view.get_file() diff --git a/django_downloadview/views/base.py b/django_downloadview/views/base.py index 807c0c9..809f8c4 100644 --- a/django_downloadview/views/base.py +++ b/django_downloadview/views/base.py @@ -1,10 +1,11 @@ # -*- coding: utf-8 -*- """Base material for download views: :class:`DownloadMixin` and :class:`BaseDownloadView`""" -from django.http import HttpResponseNotModified +from django.http import HttpResponseNotModified, Http404 from django.views.generic.base import View from django.views.static import was_modified_since +from django_downloadview import exceptions from django_downloadview.response import DownloadResponse @@ -33,7 +34,12 @@ class DownloadMixin(object): basename = None def get_file(self): - """Return a file wrapper instance.""" + """Return a file wrapper instance. + + Raises :class:`~django_downloadview.exceptions.FileNotFound` if file + does not exist. + + """ raise NotImplementedError() def get_basename(self): @@ -78,8 +84,14 @@ class DownloadMixin(object): response = self.response_class(*response_args, **response_kwargs) return response + def file_not_found_response(self): + """Raise Http404.""" + raise Http404() + def render_to_response(self, *response_args, **response_kwargs): - """Return "download" response. + """Return "download" response (if everything is ok). + + Return :meth:`file_not_found_response` if file does not exist. Respects the "HTTP_IF_MODIFIED_SINCE" header if any. In that case, uses :py:meth:`was_modified_since` and :py:meth:`not_modified_response`. @@ -87,7 +99,10 @@ class DownloadMixin(object): Else, uses :py:meth:`download_response` to return a download response. """ - self.file_instance = self.get_file() + try: + self.file_instance = self.get_file() + except exceptions.FileNotFound: + return self.file_not_found_response() # Respect the If-Modified-Since header. since = self.request.META.get('HTTP_IF_MODIFIED_SINCE', None) if since is not None: diff --git a/django_downloadview/views/object.py b/django_downloadview/views/object.py index 43f3127..eadf772 100644 --- a/django_downloadview/views/object.py +++ b/django_downloadview/views/object.py @@ -2,25 +2,33 @@ """Stream files that live in models.""" from django.views.generic.detail import SingleObjectMixin +from django_downloadview.exceptions import FileNotFound from django_downloadview.views.base import BaseDownloadView class ObjectDownloadView(SingleObjectMixin, BaseDownloadView): """Serve file fields from models. - This class extends BaseDetailView, so you can use its arguments to target - the instance to operate on: slug, slug_kwarg, model, queryset... - See Django's DetailView reference for details. + This class extends :class:`~django.views.generic.detail.SingleObjectMixin`, + so you can use its arguments to target the instance to operate on: + ``slug``, ``slug_kwarg``, ``model``, ``queryset``... - In addition to BaseDetailView arguments, you can set arguments related to - the file to be downloaded. + In addition to :class:`~django.views.generic.detail.SingleObjectMixin` + arguments, you can set arguments related to the file to be downloaded: - The main one is ``file_field``. + * :attr:`file_field`; + * :attr:`basename_field`; + * :attr:`encoding_field`; + * :attr:`mime_type_field`; + * :attr:`charset_field`; + * :attr:`modification_time_field`; + * :attr:`size_field`. - The other arguments are provided for convenience, in case your model holds - some (deserialized) metadata about the file, such as its basename, its - modification time, its MIME type... These fields may be particularly handy - if your file storage is not the local filesystem. + :attr:`file_field` is the main one. Other arguments are provided for + convenience, in case your model holds some (deserialized) metadata about + the file, such as its basename, its modification time, its MIME type... + These fields may be particularly handy if your file storage is not the + local filesystem. """ #: Name of the model's attribute which contains the file to be streamed. @@ -58,7 +66,18 @@ class ObjectDownloadView(SingleObjectMixin, BaseDownloadView): :attr:`size` are configured. """ - file_instance = getattr(self.object, self.file_field) + try: + file_instance = getattr(self.object, self.file_field) + except AttributeError: + raise FileNotFound('Failed to retrieve file from field="{field}" ' + 'on object="{object}"'.format( + field=self.file_field, + object=self.object)) + if not file_instance: + raise FileNotFound('Field="{field}" on object="{object}" is ' + 'empty'.format( + field=self.file_field, + object=self.object)) for field in ('encoding', 'mime_type', 'charset', 'modification_time', 'size'): model_field = getattr(self, '%s_field' % field, False)